Skip to content

Don't safeguard empty strings while reading #1223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ax3l merged 3 commits into from
Mar 14, 2022
Merged

Don't safeguard empty strings while reading #1223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb33694
Don't safeguard empty strings while reading
franzpoeschel Mar 9, 2022
1be5918
Update TODO Comment
ax3l Mar 14, 2022
ebb77e6
Add missing update for VEC_CLONG_DOUBLE
ax3l Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 54 additions & 11 deletions include/openPMD/backend/Attributable.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,37 @@ namespace internal
A_MAP m_attributes;
};

enum class SetAttributeMode : char
{
WhileReadingAttributes,
FromPublicAPICall
Copy link
Member

@ax3l ax3l Mar 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that public API call both read and write? Might need a doxygen string, even though its readable already :)

Copy link
Member

@ax3l ax3l Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, Set... so write

Copy link
Contributor Author

@franzpoeschel franzpoeschel Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's write-only in the public API, but during parsing, we emulate read_write mode and use setAttribute also internally

};

/** Verify values of attributes in the frontend
*
* verify string attributes are not empty (backend restriction, e.g., HDF5)
*/
template <typename T>
inline void attr_value_check(std::string const /* key */, T /* value */)
inline void attr_value_check(
std::string const /* key */, T /* value */, SetAttributeMode)
{}

template <>
inline void attr_value_check(std::string const key, std::string const value)
inline void attr_value_check(
std::string const key, std::string const value, SetAttributeMode mode)
{
if (value.empty())
throw std::runtime_error(
"[setAttribute] Value for string attribute '" + key +
"' must not be empty!");
switch (mode)
{
case SetAttributeMode::FromPublicAPICall:
if (value.empty())
throw std::runtime_error(
"[setAttribute] Value for string attribute '" + key +
"' must not be empty!");
break;
case SetAttributeMode::WhileReadingAttributes:
// no checks while reading
break;
}
}

template <typename>
Expand Down Expand Up @@ -276,6 +292,13 @@ OPENPMD_protected
void seriesFlush(internal::FlushParams);

void flushAttributes(internal::FlushParams const &);

template <typename T>
bool setAttributeImpl(
std::string const &key, T value, internal::SetAttributeMode);
bool setAttributeImpl(
std::string const &key, char const value[], internal::SetAttributeMode);

enum ReadMode
{
/**
Expand Down Expand Up @@ -410,11 +433,29 @@ OPENPMD_protected
virtual void linkHierarchy(Writable &w);
}; // Attributable

// TODO explicitly instantiate Attributable::setAttribute for all T in Datatype
template <typename T>
inline bool Attributable::setAttribute(std::string const &key, T value)
{
internal::attr_value_check(key, value);
return setAttributeImpl(
key, std::move(value), internal::SetAttributeMode::FromPublicAPICall);
}

inline bool
Attributable::setAttribute(std::string const &key, char const value[])
{
return setAttributeImpl(
key, value, internal::SetAttributeMode::FromPublicAPICall);
}

// note: we explicitly instantiate Attributable::setAttributeImpl for all T in
// Datatype in Attributable.cpp
template <typename T>
inline bool Attributable::setAttributeImpl(
std::string const &key,
T value,
internal::SetAttributeMode setAttributeMode)
{
internal::attr_value_check(key, value, setAttributeMode);

auto &attri = get();
if (IOHandler() && Access::READ_ONLY == IOHandler()->m_frontendAccess)
Expand Down Expand Up @@ -442,10 +483,12 @@ inline bool Attributable::setAttribute(std::string const &key, T value)
}
}

inline bool
Attributable::setAttribute(std::string const &key, char const value[])
inline bool Attributable::setAttributeImpl(
std::string const &key,
char const value[],
internal::SetAttributeMode setAttributeMode)
{
return this->setAttribute(key, std::string(value));
return this->setAttributeImpl(key, std::string(value), setAttributeMode);
}

template <typename T>
Expand Down
176 changes: 140 additions & 36 deletions src/backend/Attributable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,96 +299,183 @@ void Attributable::readAttributes(ReadMode mode)
}
std::array<double, 7> arr;
std::copy_n(vector.begin(), 7, arr.begin());
setAttribute(key, std::move(arr));
setAttributeImpl(
key,
std::move(arr),
internal::SetAttributeMode::WhileReadingAttributes);
}
else
{
setAttribute(key, std::move(vector));
setAttributeImpl(
key,
std::move(vector),
internal::SetAttributeMode::WhileReadingAttributes);
}
};

switch (*aRead.dtype)
{
case DT::CHAR:
setAttribute(att, a.get<char>());
setAttributeImpl(
att,
a.get<char>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::UCHAR:
setAttribute(att, a.get<unsigned char>());
setAttributeImpl(
att,
a.get<unsigned char>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::SHORT:
setAttribute(att, a.get<short>());
setAttributeImpl(
att,
a.get<short>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::INT:
setAttribute(att, a.get<int>());
setAttributeImpl(
att,
a.get<int>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::LONG:
setAttribute(att, a.get<long>());
setAttributeImpl(
att,
a.get<long>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::LONGLONG:
setAttribute(att, a.get<long long>());
setAttributeImpl(
att,
a.get<long long>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::USHORT:
setAttribute(att, a.get<unsigned short>());
setAttributeImpl(
att,
a.get<unsigned short>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::UINT:
setAttribute(att, a.get<unsigned int>());
setAttributeImpl(
att,
a.get<unsigned int>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::ULONG:
setAttribute(att, a.get<unsigned long>());
setAttributeImpl(
att,
a.get<unsigned long>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::ULONGLONG:
setAttribute(att, a.get<unsigned long long>());
setAttributeImpl(
att,
a.get<unsigned long long>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::FLOAT:
setAttribute(att, a.get<float>());
setAttributeImpl(
att,
a.get<float>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::DOUBLE:
setAttribute(att, a.get<double>());
setAttributeImpl(
att,
a.get<double>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::LONG_DOUBLE:
setAttribute(att, a.get<long double>());
setAttributeImpl(
att,
a.get<long double>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::CFLOAT:
setAttribute(att, a.get<std::complex<float> >());
setAttributeImpl(
att,
a.get<std::complex<float> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::CDOUBLE:
setAttribute(att, a.get<std::complex<double> >());
setAttributeImpl(
att,
a.get<std::complex<double> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::CLONG_DOUBLE:
setAttribute(att, a.get<std::complex<long double> >());
setAttributeImpl(
att,
a.get<std::complex<long double> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::STRING:
setAttribute(att, a.get<std::string>());
setAttributeImpl(
att,
a.get<std::string>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_CHAR:
setAttribute(att, a.get<std::vector<char> >());
setAttributeImpl(
att,
a.get<std::vector<char> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_SHORT:
setAttribute(att, a.get<std::vector<short> >());
setAttributeImpl(
att,
a.get<std::vector<short> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_INT:
setAttribute(att, a.get<std::vector<int> >());
setAttributeImpl(
att,
a.get<std::vector<int> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_LONG:
setAttribute(att, a.get<std::vector<long> >());
setAttributeImpl(
att,
a.get<std::vector<long> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_LONGLONG:
setAttribute(att, a.get<std::vector<long long> >());
setAttributeImpl(
att,
a.get<std::vector<long long> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_UCHAR:
setAttribute(att, a.get<std::vector<unsigned char> >());
setAttributeImpl(
att,
a.get<std::vector<unsigned char> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_USHORT:
setAttribute(att, a.get<std::vector<unsigned short> >());
setAttributeImpl(
att,
a.get<std::vector<unsigned short> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_UINT:
setAttribute(att, a.get<std::vector<unsigned int> >());
setAttributeImpl(
att,
a.get<std::vector<unsigned int> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_ULONG:
setAttribute(att, a.get<std::vector<unsigned long> >());
setAttributeImpl(
att,
a.get<std::vector<unsigned long> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_ULONGLONG:
setAttribute(att, a.get<std::vector<unsigned long long> >());
setAttributeImpl(
att,
a.get<std::vector<unsigned long long> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_FLOAT:
guardUnitDimension(att, a.get<std::vector<float> >());
Expand All @@ -400,23 +487,40 @@ void Attributable::readAttributes(ReadMode mode)
guardUnitDimension(att, a.get<std::vector<long double> >());
break;
case DT::VEC_CFLOAT:
setAttribute(att, a.get<std::vector<std::complex<float> > >());
setAttributeImpl(
att,
a.get<std::vector<std::complex<float> > >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_CDOUBLE:
setAttribute(att, a.get<std::vector<std::complex<double> > >());
setAttributeImpl(
att,
a.get<std::vector<std::complex<double> > >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_CLONG_DOUBLE:
setAttribute(
att, a.get<std::vector<std::complex<long double> > >());
setAttributeImpl(
att,
a.get<std::vector<std::complex<long double> > >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::VEC_STRING:
setAttribute(att, a.get<std::vector<std::string> >());
setAttributeImpl(
att,
a.get<std::vector<std::string> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::ARR_DBL_7:
setAttribute(att, a.get<std::array<double, 7> >());
setAttributeImpl(
att,
a.get<std::array<double, 7> >(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::BOOL:
setAttribute(att, a.get<bool>());
setAttributeImpl(
att,
a.get<bool>(),
internal::SetAttributeMode::WhileReadingAttributes);
break;
case DT::UNDEFINED:
throw std::runtime_error("Invalid Attribute datatype during read");
Expand Down