From 08a24ba2cf5007a2639035cc9661e18f3637223e Mon Sep 17 00:00:00 2001 From: Maxim Smolskiy Date: Tue, 28 Jan 2025 11:08:26 +0300 Subject: [PATCH] Merge pull request #26846 from MaximSmolskiy:fix_bug_with_int64_support_for_FileStorage Fix bug with int64 support for FileStorage #26846 ### Pull Request Readiness Checklist Fix #26829, https://github.com/opencv/opencv-python/issues/1078 In current implementation of `int64` support raw size of recorded integer is variable (`4` or `8` bytes depending on value). But then we iterate over nodes we need to know it exact value https://github.com/opencv/opencv/blob/dfad11aae7ef3b3a0643379266bc363b1a9c3d40/modules/core/src/persistence.cpp#L2596-L2609 Bug is that `rawSize` method still return `4` for any integer. I haven't figured out a way how to get variable raw size for integer in this method. I made raw size for integer is constant and equal to `8`. Yes, after this patch memory consumption for integers will increase, but I don't know a better way to do it yet. At least this fixes bug and implementation becomes more correct See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake --- modules/core/src/persistence.cpp | 15 ++------- modules/core/test/test_io.cpp | 58 ++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/modules/core/src/persistence.cpp b/modules/core/src/persistence.cpp index 9bb5f0ab6f..7d1f554054 100644 --- a/modules/core/src/persistence.cpp +++ b/modules/core/src/persistence.cpp @@ -2441,7 +2441,7 @@ size_t FileNode::rawSize() const p += 4; size_t sz0 = (size_t)(p - p0); if( tp == INT ) - return sz0 + 4; + return sz0 + 8; if( tp == REAL ) return sz0 + 8; if( tp == NONE ) @@ -2475,13 +2475,7 @@ void FileNode::setValue( int type, const void* value, int len ) sz += 4; if( type == INT ) - { - int64_t ival = *(const int64_t*)value; - if (ival > INT_MAX || ival < INT_MIN) - sz += 8; - else - sz += 4; - } + sz += 8; else if( type == REAL ) sz += 8; else if( type == STRING ) @@ -2502,10 +2496,7 @@ void FileNode::setValue( int type, const void* value, int len ) if( type == INT ) { int64_t ival = *(const int64_t*)value; - if (sz > 8) - writeInt(p, ival); - else - writeInt(p, static_cast(ival)); + writeInt(p, ival); } else if( type == REAL ) { diff --git a/modules/core/test/test_io.cpp b/modules/core/test/test_io.cpp index 99972b964c..5ec56d89ab 100644 --- a/modules/core/test/test_io.cpp +++ b/modules/core/test/test_io.cpp @@ -2003,6 +2003,64 @@ TEST(Core_InputOutput, FileStorage_invalid_attribute_value_regression_25946) ASSERT_EQ(0, std::remove(fileName.c_str())); } +// see https://github.com/opencv/opencv/issues/26829 +TEST(Core_InputOutput, FileStorage_int64_26829) +{ + String content = + "%YAML:1.0\n" + "String1: string1\n" + "IntMin: -2147483648\n" + "String2: string2\n" + "Int64Min: -9223372036854775808\n" + "String3: string3\n" + "IntMax: 2147483647\n" + "String4: string4\n" + "Int64Max: 9223372036854775807\n" + "String5: string5\n"; + + FileStorage fs(content, FileStorage::READ | FileStorage::MEMORY); + + { + std::string str; + + fs["String1"] >> str; + EXPECT_EQ(str, "string1"); + + fs["String2"] >> str; + EXPECT_EQ(str, "string2"); + + fs["String3"] >> str; + EXPECT_EQ(str, "string3"); + + fs["String4"] >> str; + EXPECT_EQ(str, "string4"); + + fs["String5"] >> str; + EXPECT_EQ(str, "string5"); + } + + { + int value; + + fs["IntMin"] >> value; + EXPECT_EQ(value, INT_MIN); + + fs["IntMax"] >> value; + EXPECT_EQ(value, INT_MAX); + } + + + { + int64_t value; + + fs["Int64Min"] >> value; + EXPECT_EQ(value, INT64_MIN); + + fs["Int64Max"] >> value; + EXPECT_EQ(value, INT64_MAX); + } +} + template T fsWriteRead(const T& expectedValue, const char* ext) {