From 5b75352cecb07fc360602b8222d92d8b385e1bad Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Tue, 5 Mar 2024 21:08:34 +0530 Subject: [PATCH 01/12] adding error msg issue #23190 --- src/core/src/pass/serialize.cpp | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 1b45730c2ed7db..8c88afbb4ab350 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1229,21 +1229,27 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { if (xmlDir != m_xmlPath) ov::util::create_directory_recursive(xmlDir); - std::ofstream bin_file(m_binPath, std::ios::out | std::ios::binary); - OPENVINO_ASSERT(bin_file, "Can't open bin file: \"" + m_binPath + "\""); - - // create xml file - std::ofstream xml_file(m_xmlPath, std::ios::out); - OPENVINO_ASSERT(xml_file, "Can't open xml file: \"" + m_xmlPath + "\""); - try { + // create bin file + std::ofstream bin_file(m_binPath, std::ios::out | std::ios::binary); + bin_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); + OPENVINO_ASSERT(bin_file, "Can't open bin file: \"" + m_binPath + "\""); + + // create xml file + std::ofstream xml_file(m_xmlPath, std::ios::out); + xml_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); + OPENVINO_ASSERT(xml_file, "Can't open xml file: \"" + m_xmlPath + "\""); + serializeFunc(xml_file, bin_file, model, m_version); - } catch (const ov::AssertFailure&) { - // optimization decision was made to create .bin file upfront and - // write to it directly instead of buffering its content in memory, - // hence we need to delete it here in case of failure - xml_file.close(); - bin_file.close(); + } catch (const std::ofstream::failure& e) { + // Handle file stream errors + std::cerr << "Exception opening/writing file. Not Enough Space in disk: " << e.what() << '\n'; + std::remove(m_xmlPath.c_str()); + std::remove(m_binPath.c_str()); + throw; + } catch (const ov::AssertFailure& e) { + // Handle other any exceptions + std::cerr << "OpenVINO assertion failed: " << e.what() << '\n'; std::remove(m_xmlPath.c_str()); std::remove(m_binPath.c_str()); throw; From abca81bfab53d5ae74685161a5bf59f6de04aa3d Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Wed, 6 Mar 2024 20:23:35 +0530 Subject: [PATCH 02/12] Droped catch block for ofstream --- src/core/src/pass/serialize.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 8c88afbb4ab350..05a6dc88cfee13 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1241,14 +1241,8 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { OPENVINO_ASSERT(xml_file, "Can't open xml file: \"" + m_xmlPath + "\""); serializeFunc(xml_file, bin_file, model, m_version); - } catch (const std::ofstream::failure& e) { - // Handle file stream errors - std::cerr << "Exception opening/writing file. Not Enough Space in disk: " << e.what() << '\n'; - std::remove(m_xmlPath.c_str()); - std::remove(m_binPath.c_str()); - throw; } catch (const ov::AssertFailure& e) { - // Handle other any exceptions + // Handle exceptions std::cerr << "OpenVINO assertion failed: " << e.what() << '\n'; std::remove(m_xmlPath.c_str()); std::remove(m_binPath.c_str()); From 1b79745b5c98a91fe26cbd78b83bdbd28f15d76d Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Wed, 20 Mar 2024 08:15:21 +0530 Subject: [PATCH 03/12] made changes in serialize.cpp, added test --- src/core/src/pass/serialize.cpp | 11 ++--- .../tests/pass/serialization/serialize.cpp | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 931db4da595f6f..79c6cd40bec9ba 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1095,7 +1095,7 @@ void ngfunction_2_ir(pugi::xml_node& netXml, } // fill general attributes - { + { bool compress_to_fp16 = false; ov::element::Type output_element_type = ov::element::dynamic; if (is_fp16_compression_postponed(node->get_rt_info())) { @@ -1231,19 +1231,20 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { try { // create bin file - std::ofstream bin_file(m_binPath, std::ios::out | std::ios::binary); + std::ofstream bin_file; bin_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); + bin_file.open(m_binPath, std::ios::out | std::ios::binary); OPENVINO_ASSERT(bin_file, "Can't open bin file: \"" + m_binPath + "\""); // create xml file - std::ofstream xml_file(m_xmlPath, std::ios::out); + std::ofstream xml_file; xml_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); + xml_file.open(m_xmlPath, std::ios::out); OPENVINO_ASSERT(xml_file, "Can't open xml file: \"" + m_xmlPath + "\""); serializeFunc(xml_file, bin_file, model, m_version); } catch (const ov::AssertFailure& e) { - // Handle exceptions - std::cerr << "OpenVINO assertion failed: " << e.what() << '\n'; + std::cerr << "OpenVINO assertion failed: " << '\n'; std::remove(m_xmlPath.c_str()); std::remove(m_binPath.c_str()); throw; diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index e45d5d1d1434ff..ed7656a1c33b9a 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -14,6 +14,25 @@ #include "openvino/util/file_util.hpp" #include "read_ir.hpp" +#ifdef _WIN32 +#include +#else +#include +#endif + +bool createReadOnlyFile(const std::string& filename) { + std::ofstream file(filename); + if(file.is_open()) { + file.close(); +#ifdef _WIN32 + return SetFileAttributes(filename.c_str(), FILE_ATTRIBUTE_READONLY) != 0; +#else + return chmod(filename.c_str(), S_IRUSR) == 0; // Set file as read-only for owner +#endif + } + return false; +} + using SerializationParams = std::tuple; class SerializationTest : public ov::test::TestsCommon, public testing::WithParamInterface { @@ -56,6 +75,32 @@ class SerializationTest : public ov::test::TestsCommon, public testing::WithPara } }; +TEST(SerializationTest, WriteInReadOnly) { + // Set up paths + std::string test_xml_path = "test_file.xml"; + std::string test_bin_path = "test_file.bin"; + + // Create read-only files + ASSERT_TRUE(createReadOnlyFile(test_xml_path)); + ASSERT_TRUE(createReadOnlyFile(test_bin_path)); + + // Set up the serializer with the current paths + ov::pass::Serialize serializer(test_xml_path, test_bin_path); + + + std::shared_ptr m = std::make_shared(); + + // Expect that running the serializer on a read-only file throws an exception + EXPECT_THROW(serializer.run_on_model(m), ov::AssertFailure); + + // Confirm that files were not successfully written. + ASSERT_FALSE(std::ifstream(test_xml_path).good()); + ASSERT_FALSE(std::ifstream(test_bin_path).good()); + + // Clean up + std::remove(test_xml_path.c_str()); + std::remove(test_bin_path.c_str()); +} TEST_P(SerializationTest, CompareFunctions) { CompareSerialized([this](const std::shared_ptr& m) { ov::pass::Serialize(m_out_xml_path, m_out_bin_path).run_on_model(m); From 53ba3e1aba51ad2a1ebe3657dc17784432e99267 Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Wed, 20 Mar 2024 08:35:04 +0530 Subject: [PATCH 04/12] made changes added test --- src/core/src/pass/serialize.cpp | 7 ++++--- src/core/tests/pass/serialization/serialize.cpp | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 79c6cd40bec9ba..c0f5ed1f0d03fd 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1095,7 +1095,7 @@ void ngfunction_2_ir(pugi::xml_node& netXml, } // fill general attributes - { + { bool compress_to_fp16 = false; ov::element::Type output_element_type = ov::element::dynamic; if (is_fp16_compression_postponed(node->get_rt_info())) { @@ -1239,12 +1239,13 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { // create xml file std::ofstream xml_file; xml_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); - xml_file.open(m_xmlPath, std::ios::out); + xml_file.open(m_xmlPath, std::ios::out | std::ios::binary); OPENVINO_ASSERT(xml_file, "Can't open xml file: \"" + m_xmlPath + "\""); serializeFunc(xml_file, bin_file, model, m_version); } catch (const ov::AssertFailure& e) { - std::cerr << "OpenVINO assertion failed: " << '\n'; + // Handle exceptions + std::cerr << "OpenVINO assertion failed: " << e.what() << '\n'; std::remove(m_xmlPath.c_str()); std::remove(m_binPath.c_str()); throw; diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index ed7656a1c33b9a..ba98c8c10421a2 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -101,6 +101,7 @@ TEST(SerializationTest, WriteInReadOnly) { std::remove(test_xml_path.c_str()); std::remove(test_bin_path.c_str()); } + TEST_P(SerializationTest, CompareFunctions) { CompareSerialized([this](const std::shared_ptr& m) { ov::pass::Serialize(m_out_xml_path, m_out_bin_path).run_on_model(m); From 24dfc7c51304191308582533d322eb00dcd2be69 Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Wed, 20 Mar 2024 08:39:59 +0530 Subject: [PATCH 05/12] updated serialize.cpp --- src/core/src/pass/serialize.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index c0f5ed1f0d03fd..7365a9106bc5d9 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1245,7 +1245,6 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { serializeFunc(xml_file, bin_file, model, m_version); } catch (const ov::AssertFailure& e) { // Handle exceptions - std::cerr << "OpenVINO assertion failed: " << e.what() << '\n'; std::remove(m_xmlPath.c_str()); std::remove(m_binPath.c_str()); throw; From 592a9149a1239f6dd33abb792f9ac99ed816acf0 Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Wed, 20 Mar 2024 14:14:56 +0530 Subject: [PATCH 06/12] made review changes --- src/core/tests/pass/serialization/serialize.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index ba98c8c10421a2..e9610f1d9d78d0 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -77,8 +77,9 @@ class SerializationTest : public ov::test::TestsCommon, public testing::WithPara TEST(SerializationTest, WriteInReadOnly) { // Set up paths - std::string test_xml_path = "test_file.xml"; - std::string test_bin_path = "test_file.bin"; + std::string filePrefix = ov::test::utils::generateTestFilePrefix(); + std::string test_xml_path = filePrefix + "test_file.xml"; + std::string test_bin_path = filePrefix + "test_file.bin"; // Create read-only files ASSERT_TRUE(createReadOnlyFile(test_xml_path)); @@ -88,7 +89,7 @@ TEST(SerializationTest, WriteInReadOnly) { ov::pass::Serialize serializer(test_xml_path, test_bin_path); - std::shared_ptr m = std::make_shared(); + auto m = std::make_shared(ov::OutputVector{}, ov::ParameterVector {}, ""); // Expect that running the serializer on a read-only file throws an exception EXPECT_THROW(serializer.run_on_model(m), ov::AssertFailure); From 735f41bf059585077e93a2b5e91b7a1d70401443 Mon Sep 17 00:00:00 2001 From: Shubh Mishra <155224614+mishra-18@users.noreply.github.com> Date: Sat, 8 Jun 2024 09:43:11 +0530 Subject: [PATCH 07/12] Update src/core/tests/pass/serialization/serialize.cpp Co-authored-by: Tomasz Jankowski --- src/core/tests/pass/serialization/serialize.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index e9610f1d9d78d0..d028f46585a5ea 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -97,10 +97,6 @@ TEST(SerializationTest, WriteInReadOnly) { // Confirm that files were not successfully written. ASSERT_FALSE(std::ifstream(test_xml_path).good()); ASSERT_FALSE(std::ifstream(test_bin_path).good()); - - // Clean up - std::remove(test_xml_path.c_str()); - std::remove(test_bin_path.c_str()); } TEST_P(SerializationTest, CompareFunctions) { From d86d8e3582b866d1fb4fd030a4c49b1bfc2b3956 Mon Sep 17 00:00:00 2001 From: Shubh Mishra <155224614+mishra-18@users.noreply.github.com> Date: Sat, 8 Jun 2024 09:43:37 +0530 Subject: [PATCH 08/12] Update src/core/tests/pass/serialization/serialize.cpp Co-authored-by: Tomasz Jankowski --- src/core/tests/pass/serialization/serialize.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index d028f46585a5ea..6815a2a0ed97ac 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -76,11 +76,6 @@ class SerializationTest : public ov::test::TestsCommon, public testing::WithPara }; TEST(SerializationTest, WriteInReadOnly) { - // Set up paths - std::string filePrefix = ov::test::utils::generateTestFilePrefix(); - std::string test_xml_path = filePrefix + "test_file.xml"; - std::string test_bin_path = filePrefix + "test_file.bin"; - // Create read-only files ASSERT_TRUE(createReadOnlyFile(test_xml_path)); ASSERT_TRUE(createReadOnlyFile(test_bin_path)); From 22c16f674ffd035b92ee45fc12b88dea0ed31897 Mon Sep 17 00:00:00 2001 From: Shubh Mishra <155224614+mishra-18@users.noreply.github.com> Date: Sat, 8 Jun 2024 09:43:51 +0530 Subject: [PATCH 09/12] Update src/core/tests/pass/serialization/serialize.cpp Co-authored-by: Tomasz Jankowski --- src/core/tests/pass/serialization/serialize.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index 6815a2a0ed97ac..d84cfc2188f973 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -20,6 +20,7 @@ #include #endif +namespace { bool createReadOnlyFile(const std::string& filename) { std::ofstream file(filename); if(file.is_open()) { @@ -32,6 +33,7 @@ bool createReadOnlyFile(const std::string& filename) { } return false; } +} using SerializationParams = std::tuple; From 6b8f83b876cfab6a43c4113ef19c53437586c8c8 Mon Sep 17 00:00:00 2001 From: Shubh Mishra <155224614+mishra-18@users.noreply.github.com> Date: Mon, 10 Jun 2024 18:11:55 +0530 Subject: [PATCH 10/12] Updated TEST in serialize.cpp --- src/core/tests/pass/serialization/serialize.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index d84cfc2188f973..6ac253dfa88fa8 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -79,11 +79,11 @@ class SerializationTest : public ov::test::TestsCommon, public testing::WithPara TEST(SerializationTest, WriteInReadOnly) { // Create read-only files - ASSERT_TRUE(createReadOnlyFile(test_xml_path)); - ASSERT_TRUE(createReadOnlyFile(test_bin_path)); + ASSERT_TRUE(createReadOnlyFile(m_out_xml_path)); + ASSERT_TRUE(createReadOnlyFile(m_out_bin_path)); // Set up the serializer with the current paths - ov::pass::Serialize serializer(test_xml_path, test_bin_path); + ov::pass::Serialize serializer(m_out_xml_path, m_out_bin_path); auto m = std::make_shared(ov::OutputVector{}, ov::ParameterVector {}, ""); @@ -92,8 +92,8 @@ TEST(SerializationTest, WriteInReadOnly) { EXPECT_THROW(serializer.run_on_model(m), ov::AssertFailure); // Confirm that files were not successfully written. - ASSERT_FALSE(std::ifstream(test_xml_path).good()); - ASSERT_FALSE(std::ifstream(test_bin_path).good()); + ASSERT_FALSE(std::ifstream(m_out_xml_path).good()); + ASSERT_FALSE(std::ifstream(m_out_bin_path).good()); } TEST_P(SerializationTest, CompareFunctions) { From 7be4be595122af1164d15370261dd1903eab6206 Mon Sep 17 00:00:00 2001 From: Shubh Mishra <155224614+mishra-18@users.noreply.github.com> Date: Tue, 11 Jun 2024 20:17:07 +0530 Subject: [PATCH 11/12] Update serialize.cpp --- src/core/tests/pass/serialization/serialize.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index 6ac253dfa88fa8..e364d728c02415 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -15,15 +15,15 @@ #include "read_ir.hpp" #ifdef _WIN32 -#include +# include #else -#include +# include #endif namespace { bool createReadOnlyFile(const std::string& filename) { std::ofstream file(filename); - if(file.is_open()) { + if (file.is_open()) { file.close(); #ifdef _WIN32 return SetFileAttributes(filename.c_str(), FILE_ATTRIBUTE_READONLY) != 0; @@ -33,7 +33,7 @@ bool createReadOnlyFile(const std::string& filename) { } return false; } -} +} // namespace using SerializationParams = std::tuple; @@ -84,9 +84,8 @@ TEST(SerializationTest, WriteInReadOnly) { // Set up the serializer with the current paths ov::pass::Serialize serializer(m_out_xml_path, m_out_bin_path); - - auto m = std::make_shared(ov::OutputVector{}, ov::ParameterVector {}, ""); + auto m = std::make_shared(ov::OutputVector{}, ov::ParameterVector{}, ""); // Expect that running the serializer on a read-only file throws an exception EXPECT_THROW(serializer.run_on_model(m), ov::AssertFailure); From 17685253fd5c5401a1fa25ebc9ac4da8cd1a7d0b Mon Sep 17 00:00:00 2001 From: mishra-18 Date: Mon, 8 Jul 2024 15:07:13 +0530 Subject: [PATCH 12/12] ran through clang-format --- src/core/src/pass/serialize.cpp | 6 +++--- src/core/tests/pass/serialization/serialize.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 5376f56b9c6fed..d7ccd9d185d274 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1220,15 +1220,15 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { auto xmlDir = ov::util::get_directory(xmlPath_ref); if (xmlDir != xmlPath_ref) ov::util::create_directory_recursive(xmlDir); - + try { // create bin file std::ofstream bin_file; bin_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); bin_file.open(m_binPath, std::ios::out | std::ios::binary); OPENVINO_ASSERT(bin_file, "Can't open bin file: \"" + m_binPath + "\""); - - // create xml file + + // create xml file std::ofstream xml_file; xml_file.exceptions(std::ofstream::failbit | std::ofstream::badbit); xml_file.open(m_xmlPath, std::ios::out | std::ios::binary); diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index e364d728c02415..fb7b9729a8589b 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -33,7 +33,7 @@ bool createReadOnlyFile(const std::string& filename) { } return false; } -} // namespace +} // namespace using SerializationParams = std::tuple; @@ -84,7 +84,7 @@ TEST(SerializationTest, WriteInReadOnly) { // Set up the serializer with the current paths ov::pass::Serialize serializer(m_out_xml_path, m_out_bin_path); - + auto m = std::make_shared(ov::OutputVector{}, ov::ParameterVector{}, ""); // Expect that running the serializer on a read-only file throws an exception