Skip to content

Commit

Permalink
make struct size parameter compulsory to ensure padding correctness
Browse files Browse the repository at this point in the history
  • Loading branch information
JasonRuonanWang committed May 4, 2022
1 parent 445f4ed commit a1672d1
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 24 deletions.
4 changes: 2 additions & 2 deletions bindings/CXX11/adios2/cxx11/ADIOS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ Operator ADIOS::InquireOperator(const std::string name)
}
}

StructDefinition ADIOS::DefineStruct(const std::string &name)
StructDefinition ADIOS::DefineStruct(const std::string &name, const size_t size)
{
CheckPointer("for struct name " + name +
", in call to ADIOS::DefineStruct");
return StructDefinition(&m_ADIOS->DefineStruct(name));
return StructDefinition(&m_ADIOS->DefineStruct(name, size));
}

StructDefinition ADIOS::InquireStruct(const std::string &name)
Expand Down
2 changes: 1 addition & 1 deletion bindings/CXX11/adios2/cxx11/ADIOS.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class ADIOS
*/
Operator InquireOperator(const std::string name);

StructDefinition DefineStruct(const std::string &name);
StructDefinition DefineStruct(const std::string &name, const size_t size);
StructDefinition InquireStruct(const std::string &name);

/**
Expand Down
6 changes: 4 additions & 2 deletions source/adios2/core/ADIOS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,15 +194,17 @@ ADIOS::InquireOperator(const std::string &name) noexcept
}
}

StructDefinition &ADIOS::DefineStruct(const std::string &name)
StructDefinition &ADIOS::DefineStruct(const std::string &name,
const size_t size)
{
if (m_StructDefinitions.find(name) != m_StructDefinitions.end())
{
helper::Throw<std::invalid_argument>("Core", "ADIOS", "CheckOperator",
"Struct " + name +
" defined twice");
}
return m_StructDefinitions[name];
return m_StructDefinitions.emplace(name, StructDefinition(size))
.first->second;
}

StructDefinition *ADIOS::InquireStruct(const std::string &name)
Expand Down
2 changes: 1 addition & 1 deletion source/adios2/core/ADIOS.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class ADIOS
std::pair<std::string, Params> *
InquireOperator(const std::string &name) noexcept;

StructDefinition &DefineStruct(const std::string &name);
StructDefinition &DefineStruct(const std::string &name, const size_t size);
StructDefinition *InquireStruct(const std::string &name);

/**
Expand Down
6 changes: 3 additions & 3 deletions source/adios2/core/IO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -964,10 +964,10 @@ VariableStruct *IO::InquireStructVariable(const std::string &name,
return ret;
}

StructDefinition &IO::DefineStruct(const std::string &name)
StructDefinition &IO::DefineStruct(const std::string &name, const size_t size)
{
m_StructDefinitions[name] = StructDefinition();
return m_StructDefinitions[name];
return m_StructDefinitions.emplace(name, StructDefinition(size))
.first->second;
}

StructDefinition *IO::InquireStruct(const std::string &name)
Expand Down
2 changes: 1 addition & 1 deletion source/adios2/core/IO.h
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ class IO
/** Inform about computation block through User->ADIOS */
void ExitComputationBlock() noexcept;

StructDefinition &DefineStruct(const std::string &name);
StructDefinition &DefineStruct(const std::string &name, const size_t size);
StructDefinition *InquireStruct(const std::string &name);

private:
Expand Down
7 changes: 3 additions & 4 deletions source/adios2/core/VariableStruct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace adios2
namespace core
{

void StructDefinition::SetStructSize(const size_t size) { m_StructSize = size; }
StructDefinition::StructDefinition(const size_t size) : m_StructSize(size) {}

void StructDefinition::AddItem(const std::string &name, const size_t offset,
const DataType type, const size_t size)
Expand All @@ -28,19 +28,18 @@ void StructDefinition::AddItem(const std::string &name, const size_t offset,
"VariableStruct::StructDefinition",
"AddItem", "invalid data type");
}
if (offset < m_StructSize)
if (offset + helper::GetDataTypeSize(type) * size > m_StructSize)
{
helper::Throw<std::invalid_argument>("core",
"VariableStruct::StructDefinition",
"AddItem", "invalid offset");
"AddItem", "exceeded struct size");
}
m_Definition.emplace_back();
auto &d = m_Definition.back();
d.Name = name;
d.Offset = offset;
d.Type = type;
d.Size = size;
m_StructSize = offset + helper::GetDataTypeSize(type) * size;
}

size_t StructDefinition::StructSize() const noexcept { return m_StructSize; }
Expand Down
2 changes: 1 addition & 1 deletion source/adios2/core/VariableStruct.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class StructDefinition
size_t Size;
};

void SetStructSize(const size_t size);
StructDefinition(const size_t size);
void AddItem(const std::string &name, const size_t offset,
const DataType type, const size_t size = 1);
size_t StructSize() const noexcept;
Expand Down
3 changes: 1 addition & 2 deletions source/adios2/engine/ssc/SscHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ void DeserializeVariable(const Buffer &input, const ShapeID shapeId,
}
if (b.shapeId == ShapeID::GlobalArray)
{
auto def = io.DefineStruct(b.name);
def.SetStructSize(b.elementSize);
auto def = io.DefineStruct(b.name, b.elementSize);
io.DefineStructVariable(b.name, def, vShape, vStart,
vShape);
}
Expand Down
10 changes: 5 additions & 5 deletions testing/adios2/engine/ssc/TestSscStruct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ void Writer(const Dims &shape, const Dims &start, const Dims &count,
auto varIntScalar = io.DefineVariable<int>("varIntScalar");
auto varString = io.DefineVariable<std::string>("varString");

auto particleDef = adios.DefineStruct("particle");
particleDef.AddItem("a", 0, adios2::DataType::Int8, 1);
particleDef.AddItem("b", 4, adios2::DataType::Int32, 4);
auto varStruct =
io.DefineStructVariable("particles", particleDef, shape, start, count);
struct particle
{
int8_t a;
int b[4];
};
auto particleDef = adios.DefineStruct("particle", sizeof(particle));
particleDef.AddItem("a", 0, adios2::DataType::Int8, 1);
particleDef.AddItem("b", 4, adios2::DataType::Int32, 4);
auto varStruct =
io.DefineStructVariable("particles", particleDef, shape, start, count);
std::vector<particle> myParticles(datasize);
for (size_t i = 0; i < datasize; ++i)
{
Expand Down
4 changes: 2 additions & 2 deletions testing/adios2/interface/TestADIOSDefineVariable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,11 +661,11 @@ TEST_F(ADIOSDefineVariableTest, DefineStructVariable)
const adios2::Dims start = {0};
const adios2::Dims count = {10};

auto def1 = adios.DefineStruct("def1");
auto def1 = adios.DefineStruct("def1", 24);
def1.AddItem("a", 0, adios2::DataType::Int8, 1);
def1.AddItem("b", 4, adios2::DataType::Int32, 5);

auto def2 = adios.DefineStruct("def2");
auto def2 = adios.DefineStruct("def2", 28);
def2.AddItem("a", 0, adios2::DataType::Int8, 1);
def2.AddItem("b", 4, adios2::DataType::Int32, 5);
def2.AddItem("c", 24, adios2::DataType::Int32);
Expand Down

0 comments on commit a1672d1

Please sign in to comment.