From 9433cf662502afb6295837763e69c5ebe74d1974 Mon Sep 17 00:00:00 2001 From: Stefano Date: Thu, 4 Jul 2024 19:00:37 +0200 Subject: [PATCH 1/6] Added error message in case the file to open does not exist --- src/File.cpp | 7 +++++++ test/FileUnitTest.cpp | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/src/File.cpp b/src/File.cpp index 40426d1..afdfa58 100644 --- a/src/File.cpp +++ b/src/File.cpp @@ -124,6 +124,13 @@ void matioCpp::File::close() bool matioCpp::File::open(const std::string &name, matioCpp::FileMode mode) { + struct stat info; + + if (stat(name.c_str(), &info) != 0) + { + std::cerr << "[ERROR][matioCpp::File::open] The file " << name << " does not exists." << std::endl; + return false; + } int matio_mode = mode == matioCpp::FileMode::ReadOnly ? mat_acc::MAT_ACC_RDONLY : mat_acc::MAT_ACC_RDWR; m_pimpl->reset(Mat_Open(name.c_str(), matio_mode), mode); return isOpen(); diff --git a/test/FileUnitTest.cpp b/test/FileUnitTest.cpp index 1d36df1..514173b 100644 --- a/test/FileUnitTest.cpp +++ b/test/FileUnitTest.cpp @@ -561,3 +561,9 @@ TEST_CASE("Write version 4") REQUIRE(boolVector(2)); REQUIRE_FALSE(boolVector(3)); } + +TEST_CASE("Open not existing file") +{ + matioCpp::File file; + REQUIRE_FALSE(file.open("notExisting.mat")); +} \ No newline at end of file From 47a6ce0aa025c1925972dae3e5254b4dbd9756ea Mon Sep 17 00:00:00 2001 From: Stefano Date: Thu, 4 Jul 2024 20:06:23 +0200 Subject: [PATCH 2/6] Moved operator[string] to variable to ease navigation of nested structs --- include/matioCpp/Struct.h | 17 +---------------- include/matioCpp/Variable.h | 20 ++++++++++++++++++++ src/Struct.cpp | 16 ---------------- src/Variable.cpp | 26 ++++++++++++++++++++++++++ test/VariableUnitTest.cpp | 21 +++++++++++++++++++++ 5 files changed, 68 insertions(+), 32 deletions(-) diff --git a/include/matioCpp/Struct.h b/include/matioCpp/Struct.h index 3d503f0..c4e57c7 100644 --- a/include/matioCpp/Struct.h +++ b/include/matioCpp/Struct.h @@ -215,23 +215,8 @@ class matioCpp::Struct : public matioCpp::Variable */ const matioCpp::Variable operator[](index_type el) const; - /** - * @brief Access field at a specific index. - * @param el The name of the field to be accessed. - * @return A Variable with a weak ownership to the underlying mat variable. This means that the data can be changed, - * but the variable cannot be resized and the name cannot change. - * @note If the field is not found, the output variable is not valid. - */ - matioCpp::Variable operator[](const std::string& el); + using Variable::operator[]; - /** - * @brief Access field at a specific index. - * @param el The name of the field to be accessed. - * @warning Each element of el has to be strictly smaller than the corresponding dimension. - * @return A const Variable with a weak ownership to the underlying mat variable. - * @note If the field is not found, the output variable is not valid. - */ - const matioCpp::Variable operator[](const std::string& el) const; }; #endif // MATIOCPP_STRUCT_H diff --git a/include/matioCpp/Variable.h b/include/matioCpp/Variable.h index 465c131..a22d48b 100644 --- a/include/matioCpp/Variable.h +++ b/include/matioCpp/Variable.h @@ -328,6 +328,26 @@ class matioCpp::Variable */ bool isValid() const; + /** + * @brief Access field at a specific index. + * @param el The name of the field to be accessed. + * @return A Variable with a weak ownership to the underlying mat variable. This means that the data can be changed, + * but the variable cannot be resized and the name cannot change. + * @note This works only if the variable is a struct. + * @note If the field is not found, the output variable is not valid. + */ + matioCpp::Variable operator[](const std::string& el); + + /** + * @brief Access field at a specific index. + * @param el The name of the field to be accessed. + * @warning Each element of el has to be strictly smaller than the corresponding dimension. + * @return A const Variable with a weak ownership to the underlying mat variable. + * @note This works only if the variable is a struct. + * @note If the field is not found, the output variable is not valid. + */ + const matioCpp::Variable operator[](const std::string& el) const; + /** * @brief Cast the variable as a Element. * diff --git a/src/Struct.cpp b/src/Struct.cpp index e29ee3b..d8ba67d 100644 --- a/src/Struct.cpp +++ b/src/Struct.cpp @@ -223,19 +223,3 @@ const matioCpp::Variable matioCpp::Struct::operator[](matioCpp::Struct::index_ty assert(el < numberOfFields() && "The specified index is out of bounds"); return getStructField(el); } - -matioCpp::Variable matioCpp::Struct::operator[](const std::string &el) -{ - size_t index = getFieldIndex(el); - assert(index < numberOfFields() && "The specified field does not exist."); - return getStructField(index); -} - -const matioCpp::Variable matioCpp::Struct::operator[](const std::string &el) const -{ - size_t index = getFieldIndex(el); - assert(index < numberOfFields() && "The specified field does not exist."); - return getStructField(index); -} - - diff --git a/src/Variable.cpp b/src/Variable.cpp index ee475d6..e887f8d 100644 --- a/src/Variable.cpp +++ b/src/Variable.cpp @@ -544,6 +544,32 @@ bool matioCpp::Variable::isValid() const return m_handler->get() && checkCompatibility(m_handler->get(), m_handler->variableType(), m_handler->valueType()); } +matioCpp::Variable matioCpp::Variable::operator[](const std::string& el) +{ + if (variableType() != matioCpp::VariableType::Struct) + { + std::cerr << "[ERROR][matioCpp::Variable::operator[]] The operator[](string) can be used only with structs." << std::endl; + assert(false); + return matioCpp::Variable(); + } + size_t index = getStructFieldIndex(el); + assert(index < getStructNumberOfFields() && "The specified field does not exist."); + return getStructField(index); +} + +const matioCpp::Variable matioCpp::Variable::operator[](const std::string& el) const +{ + if (variableType() != matioCpp::VariableType::Struct) + { + std::cerr << "[ERROR][matioCpp::Variable::operator[]] The operator[](string) can be used only with structs." << std::endl; + assert(false); + return matioCpp::Variable(); + } + size_t index = getStructFieldIndex(el); + assert(index < getStructNumberOfFields() && "The specified field does not exist."); + return getStructField(index); +} + matioCpp::CellArray matioCpp::Variable::asCellArray() { return matioCpp::CellArray(*m_handler); diff --git a/test/VariableUnitTest.cpp b/test/VariableUnitTest.cpp index ef7a449..2349d08 100644 --- a/test/VariableUnitTest.cpp +++ b/test/VariableUnitTest.cpp @@ -357,5 +357,26 @@ TEST_CASE("Conversions") } } +TEST_CASE("operator[](string)") +{ + std::vector dimensions = {1, 1}; + std::vector pointers; + pointers.emplace_back(Mat_VarDuplicate(matioCpp::Vector("vector").toMatio(), 1)); + pointers.emplace_back(Mat_VarDuplicate(matioCpp::Element("element").toMatio(), 1)); + pointers.emplace_back(Mat_VarDuplicate(matioCpp::MultiDimensionalArray("array").toMatio(), 1)); + pointers.emplace_back(Mat_VarDuplicate(matioCpp::String("name", "content").toMatio(), 1)); + pointers.emplace_back(Mat_VarDuplicate(matioCpp::Struct("otherStruct", { matioCpp::String("name", "anotherContent") }).toMatio(), 1)); + pointers.emplace_back(nullptr); + + matvar_t* matioVar = Mat_VarCreate("test", matio_classes::MAT_C_STRUCT, matio_types::MAT_T_STRUCT, 2, dimensions.data(), pointers.data(), 0); + REQUIRE(matioVar); + matioCpp::Variable sharedVar((matioCpp::SharedMatvar(matioVar))); + REQUIRE(sharedVar["name"].asString()() == "content"); + + const matioCpp::Variable& constRef = sharedVar; + REQUIRE(constRef["name"].asString()() == "content"); + + REQUIRE(sharedVar["otherStruct"]["name"].asString()() == "anotherContent"); +} From 7ba589bef51959a2ae15d0e581841e1359f5c8df Mon Sep 17 00:00:00 2001 From: Stefano Date: Thu, 4 Jul 2024 20:07:18 +0200 Subject: [PATCH 3/6] Added missing EOF --- test/FileUnitTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/FileUnitTest.cpp b/test/FileUnitTest.cpp index 514173b..dc5e4b9 100644 --- a/test/FileUnitTest.cpp +++ b/test/FileUnitTest.cpp @@ -566,4 +566,4 @@ TEST_CASE("Open not existing file") { matioCpp::File file; REQUIRE_FALSE(file.open("notExisting.mat")); -} \ No newline at end of file +} From d14b9b34505570be96213f5723199380dc6c6cfb Mon Sep 17 00:00:00 2001 From: Stefano Dafarra Date: Fri, 5 Jul 2024 09:07:03 +0200 Subject: [PATCH 4/6] Fixed documentation of operator[] Co-authored-by: Silvio Traversaro --- include/matioCpp/Variable.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/matioCpp/Variable.h b/include/matioCpp/Variable.h index a22d48b..55ffc1e 100644 --- a/include/matioCpp/Variable.h +++ b/include/matioCpp/Variable.h @@ -329,7 +329,7 @@ class matioCpp::Variable bool isValid() const; /** - * @brief Access field at a specific index. + * @brief Access field with specific name. * @param el The name of the field to be accessed. * @return A Variable with a weak ownership to the underlying mat variable. This means that the data can be changed, * but the variable cannot be resized and the name cannot change. @@ -339,7 +339,7 @@ class matioCpp::Variable matioCpp::Variable operator[](const std::string& el); /** - * @brief Access field at a specific index. + * @brief Access field with specific name. * @param el The name of the field to be accessed. * @warning Each element of el has to be strictly smaller than the corresponding dimension. * @return A const Variable with a weak ownership to the underlying mat variable. From ee32137f374fae86782937c2ecdc7a9b53abe378 Mon Sep 17 00:00:00 2001 From: Stefano Dafarra Date: Fri, 5 Jul 2024 09:25:35 +0200 Subject: [PATCH 5/6] Fixed indentation in VariableUnitTest.cpp --- test/VariableUnitTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/VariableUnitTest.cpp b/test/VariableUnitTest.cpp index 2349d08..431bf96 100644 --- a/test/VariableUnitTest.cpp +++ b/test/VariableUnitTest.cpp @@ -359,7 +359,7 @@ TEST_CASE("Conversions") TEST_CASE("operator[](string)") { - std::vector dimensions = {1, 1}; + std::vector dimensions = {1, 1}; std::vector pointers; pointers.emplace_back(Mat_VarDuplicate(matioCpp::Vector("vector").toMatio(), 1)); pointers.emplace_back(Mat_VarDuplicate(matioCpp::Element("element").toMatio(), 1)); From 554b73328e5818ea9c5cbcdb3e4733329bb07488 Mon Sep 17 00:00:00 2001 From: Stefano Date: Fri, 5 Jul 2024 13:05:41 +0200 Subject: [PATCH 6/6] Printing an error also in Release when a field is not existing Edited the way we check if a field exists to avoid multiple class to matio functions --- include/matioCpp/Variable.h | 5 +++-- src/Struct.cpp | 7 +++++-- src/StructArray.cpp | 7 +++++-- src/Variable.cpp | 32 +++++++++++++++++++++++--------- test/StructArrayUnitTest.cpp | 2 ++ 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/include/matioCpp/Variable.h b/include/matioCpp/Variable.h index 55ffc1e..b9a7666 100644 --- a/include/matioCpp/Variable.h +++ b/include/matioCpp/Variable.h @@ -127,9 +127,10 @@ class matioCpp::Variable /** * @brief Get the index of the specified field in the variable, considered as a struct * @param field The field to search - * @return The index of the field, the output of getStructNumberOfFields() if not found. + * @param index The index of the field + * @return True if successfull, false otherwise. */ - size_t getStructFieldIndex(const std::string& field) const; + bool getStructFieldIndex(const std::string& field, size_t& index) const; /** * @brief Set the field of the struct at the specified position diff --git a/src/Struct.cpp b/src/Struct.cpp index d8ba67d..9019c45 100644 --- a/src/Struct.cpp +++ b/src/Struct.cpp @@ -162,12 +162,15 @@ void matioCpp::Struct::clear() bool matioCpp::Struct::isFieldExisting(const std::string &field) const { - return getStructFieldIndex(field) < numberOfFields(); + size_t index; + return getStructFieldIndex(field, index); } size_t matioCpp::Struct::getFieldIndex(const std::string &field) const { - return getStructFieldIndex(field); + size_t index; + getStructFieldIndex(field, index); + return index; } bool matioCpp::Struct::setField(matioCpp::Struct::index_type index, const matioCpp::Variable &newValue) diff --git a/src/StructArray.cpp b/src/StructArray.cpp index 83a6f5e..2dff76f 100644 --- a/src/StructArray.cpp +++ b/src/StructArray.cpp @@ -371,12 +371,15 @@ void matioCpp::StructArray::clear() bool matioCpp::StructArray::isFieldExisting(const std::string &field) const { - return getStructFieldIndex(field) < numberOfFields(); + size_t index; + return getStructFieldIndex(field, index); } size_t matioCpp::StructArray::getFieldIndex(const std::string &field) const { - return getStructFieldIndex(field); + size_t index; + getStructFieldIndex(field, index); + return index; } bool matioCpp::StructArray::addField(const std::string &newField) diff --git a/src/Variable.cpp b/src/Variable.cpp index e887f8d..60384c2 100644 --- a/src/Variable.cpp +++ b/src/Variable.cpp @@ -232,15 +232,16 @@ char * const * matioCpp::Variable::getStructFields() const return Mat_VarGetStructFieldnames(m_handler->get()); } -size_t matioCpp::Variable::getStructFieldIndex(const std::string &field) const +bool matioCpp::Variable::getStructFieldIndex(const std::string& field, size_t& index) const { size_t i = 0; size_t numberOfFields = getStructNumberOfFields(); char * const * fields = getStructFields(); + index = numberOfFields; if (!fields) { - return numberOfFields; + return false; } while (i < numberOfFields && (strcmp(fields[i], field.c_str()) != 0)) @@ -248,7 +249,9 @@ size_t matioCpp::Variable::getStructFieldIndex(const std::string &field) const ++i; } - return i; + index = i; + + return index < numberOfFields; } bool matioCpp::Variable::setStructField(size_t index, const matioCpp::Variable &newValue, size_t structPositionInArray) @@ -312,13 +315,14 @@ bool matioCpp::Variable::setStructField(const std::string& field, const matioCpp return false; } - size_t fieldindex = getStructFieldIndex(field); + size_t fieldindex; - if ((fieldindex == getStructNumberOfFields()) && !((getArrayNumberOfElements() == 1) && addStructField(field))) + if (!getStructFieldIndex(field, fieldindex) && !((getArrayNumberOfElements() == 1) && addStructField(field))) { //This is the case when the field has not been found and, either there are more than one elements (i.e. it is part of an array), or there was an error in adding the field return false; } + //If it was not found, but the field has been added, the fieldindex is the last one return setStructField(fieldindex, newValue, structPositionInArray); } @@ -552,8 +556,13 @@ matioCpp::Variable matioCpp::Variable::operator[](const std::string& el) assert(false); return matioCpp::Variable(); } - size_t index = getStructFieldIndex(el); - assert(index < getStructNumberOfFields() && "The specified field does not exist."); + size_t index; + if (!getStructFieldIndex(el, index)) + { + std::cerr << "[ERROR][matioCpp::Variable::operator[]] The field " << el << " does not exist." << std::endl; + assert(false); + return matioCpp::Variable(); + } return getStructField(index); } @@ -565,8 +574,13 @@ const matioCpp::Variable matioCpp::Variable::operator[](const std::string& el) c assert(false); return matioCpp::Variable(); } - size_t index = getStructFieldIndex(el); - assert(index < getStructNumberOfFields() && "The specified field does not exist."); + size_t index; + if (!getStructFieldIndex(el, index)) + { + std::cerr << "[ERROR][matioCpp::Variable::operator[]] The field " << el << " does not exist." << std::endl; + assert(false); + return matioCpp::Variable(); + } return getStructField(index); } diff --git a/test/StructArrayUnitTest.cpp b/test/StructArrayUnitTest.cpp index 4189f02..7b87bb9 100644 --- a/test/StructArrayUnitTest.cpp +++ b/test/StructArrayUnitTest.cpp @@ -357,6 +357,8 @@ TEST_CASE("Assignments and modifications") size_t numberOfFields = in.numberOfFields(); REQUIRE(numberOfFields == arrayFields.size()); + REQUIRE(in.getFieldIndex("notExisting") == numberOfFields); + in.addField("addedField"); REQUIRE(in.numberOfFields() == numberOfFields + 1);