From 8e037e58e9531c85d4a3edf8bbc7012f5415d387 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Dec 2025 17:47:22 +0000 Subject: [PATCH 1/5] BUG: Fix locale-dependent parsing in NRRD reader and add test Avoid locale-dependent of floating-point metadata re: #5683, #3375, #2297 Implement locale handling in Modules/IO/NRRD/src/itkNrrdImageIO.cxx using RAII pattern. - Add ScopedCNumericLocale class for automatic locale save/restore - Apply locale protection around nrrdLoad/nrrdSave calls - Add test coverage (itkNrrdLocaleTest.cxx) Co-authored-by: Matt McCormick --- Modules/IO/NRRD/src/itkNrrdImageIO.cxx | 56 +++++++ Modules/IO/NRRD/test/CMakeLists.txt | 9 + Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx | 182 +++++++++++++++++++++ 3 files changed, 247 insertions(+) create mode 100644 Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx diff --git a/Modules/IO/NRRD/src/itkNrrdImageIO.cxx b/Modules/IO/NRRD/src/itkNrrdImageIO.cxx index e0d0825d3e0..8c000a7d948 100644 --- a/Modules/IO/NRRD/src/itkNrrdImageIO.cxx +++ b/Modules/IO/NRRD/src/itkNrrdImageIO.cxx @@ -24,9 +24,52 @@ #include "itkFloatingPointExceptions.h" #include +#include +#include +#include namespace { +// RAII class to temporarily set LC_NUMERIC locale to "C" for locale-independent +// parsing of floating-point numbers in NRRD headers. +// This prevents issues in locales that use comma as decimal separator (e.g., de_DE.UTF-8). +class ScopedCNumericLocale +{ +public: + ScopedCNumericLocale() + { + // Save current LC_NUMERIC locale + const char * currentLocale = setlocale(LC_NUMERIC, nullptr); + if (currentLocale) + { + m_SavedLocale = strdup(currentLocale); + } + // Set to C locale for parsing + setlocale(LC_NUMERIC, "C"); + } + + ~ScopedCNumericLocale() + { + // Restore original locale + if (m_SavedLocale) + { + setlocale(LC_NUMERIC, m_SavedLocale); + free(m_SavedLocale); + } + } + + // Delete copy and move operations + ScopedCNumericLocale(const ScopedCNumericLocale &) = delete; + ScopedCNumericLocale & + operator=(const ScopedCNumericLocale &) = delete; + ScopedCNumericLocale(ScopedCNumericLocale &&) = delete; + ScopedCNumericLocale & + operator=(ScopedCNumericLocale &&) = delete; + +private: + char * m_SavedLocale{ nullptr }; +}; + // This function determines which NRRD axis should be used for pixel component in the ITK image, // and which NRRD axes should be used for the ITK image axes. // The pixel component axis is the first NRRD range axis (preferably a non-list axis). @@ -395,6 +438,11 @@ NrrdImageIO::ReadImageInformation() FloatingPointExceptions::Disable(); } + // Set LC_NUMERIC to "C" locale to ensure locale-independent parsing + // of floating-point values in NRRD headers (spacing, origin, direction, etc.). + // This prevents issues in locales that use comma as decimal separator. + ScopedCNumericLocale cLocale; + // this is the mechanism by which we tell nrrdLoad to read // just the header, and none of the data nrrdIoStateSet(nio, nrrdIoStateSkipData, 1); @@ -923,6 +971,10 @@ NrrdImageIO::Read(void * buffer) FloatingPointExceptions::Disable(); #endif + // Set LC_NUMERIC to "C" locale to ensure locale-independent parsing + // of floating-point values in NRRD headers. + ScopedCNumericLocale cLocale; + // Read in the nrrd. Yes, this means that the header is being read // twice: once by NrrdImageIO::ReadImageInformation, and once here if (nrrdLoad(nrrd, this->GetFileName(), nullptr) != 0) @@ -1338,6 +1390,10 @@ NrrdImageIO::Write(const void * buffer) break; } + // Set LC_NUMERIC to "C" locale to ensure locale-independent formatting + // of floating-point values when writing NRRD headers. + ScopedCNumericLocale cLocale; + // Write the nrrd to file. if (nrrdSave(this->GetFileName(), nrrd, nio)) { diff --git a/Modules/IO/NRRD/test/CMakeLists.txt b/Modules/IO/NRRD/test/CMakeLists.txt index 004e86d1ad7..a9b34b43c47 100644 --- a/Modules/IO/NRRD/test/CMakeLists.txt +++ b/Modules/IO/NRRD/test/CMakeLists.txt @@ -17,6 +17,7 @@ set( itkNrrdVectorImageUnitLabelReadTest.cxx itkNrrd5dVectorImageReadWriteTest.cxx itkNrrdMetaDataTest.cxx + itkNrrdLocaleTest.cxx ) # For itkNrrdImageIOTest.h. @@ -356,3 +357,11 @@ itk_add_test( itkNrrdMetaDataTest ${ITK_TEST_OUTPUT_DIR} ) + +itk_add_test( + NAME itkNrrdLocaleTest + COMMAND + ITKIONRRDTestDriver + itkNrrdLocaleTest + ${ITK_TEST_OUTPUT_DIR} +) diff --git a/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx b/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx new file mode 100644 index 00000000000..8d7d304606d --- /dev/null +++ b/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx @@ -0,0 +1,182 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +#include +#include "itkImage.h" +#include "itkImageFileReader.h" +#include "itkImageFileWriter.h" +#include "itkNrrdImageIO.h" +#include "itkTestingMacros.h" + +// Test that NRRD reader handles locale-dependent parsing correctly. +// This test verifies the fix for: https://github.com/InsightSoftwareConsortium/ITK/issues/XXXX + +int +itkNrrdLocaleTest(int argc, char * argv[]) +{ + if (argc < 2) + { + std::cerr << "Missing Parameters." << std::endl; + std::cerr << "Usage: " << itkNameOfTestExecutableMacro(argv) << " OutputDirectory" << std::endl; + return EXIT_FAILURE; + } + + constexpr unsigned int Dimension = 3; + using PixelType = float; + using ImageType = itk::Image; + + // Create a test image with fractional spacing + auto image = ImageType::New(); + + ImageType::SizeType size; + size.Fill(16); + + ImageType::IndexType start; + start.Fill(0); + + ImageType::RegionType region(start, size); + image->SetRegions(region); + image->Allocate(); + image->FillBuffer(42.0f); + + // Set spacing with fractional values that would be misparsed + // in locales using comma as decimal separator + ImageType::SpacingType spacing; + spacing[0] = 3.5; // Would be parsed correctly even with truncation + spacing[1] = 0.878906; // Would become 0.0 in de_DE locale, causing error + spacing[2] = 2.2; // Would be parsed incorrectly + + image->SetSpacing(spacing); + + // Set origin with fractional values + ImageType::PointType origin; + origin[0] = 1.5; + origin[1] = 2.75; + origin[2] = 0.5; + image->SetOrigin(origin); + + // Write the image with C locale (normal operation) + std::string filename = std::string(argv[1]) + "/locale_test.nrrd"; + + using WriterType = itk::ImageFileWriter; + auto writer = WriterType::New(); + writer->SetFileName(filename); + writer->SetInput(image); + writer->SetImageIO(itk::NrrdImageIO::New()); + + ITK_TRY_EXPECT_NO_EXCEPTION(writer->Update()); + + std::cout << "Original spacing: " << spacing << std::endl; + std::cout << "Original origin: " << origin << std::endl; + + // Test 1: Read with C locale (should work) + { + setlocale(LC_NUMERIC, "C"); + + using ReaderType = itk::ImageFileReader; + auto reader = ReaderType::New(); + reader->SetFileName(filename); + reader->SetImageIO(itk::NrrdImageIO::New()); + + ITK_TRY_EXPECT_NO_EXCEPTION(reader->Update()); + + ImageType::Pointer readImage = reader->GetOutput(); + ImageType::SpacingType readSpacing = readImage->GetSpacing(); + ImageType::PointType readOrigin = readImage->GetOrigin(); + + std::cout << "C locale - Read spacing: " << readSpacing << std::endl; + std::cout << "C locale - Read origin: " << readOrigin << std::endl; + + // Verify spacing + for (unsigned int i = 0; i < Dimension; ++i) + { + if (itk::Math::abs(readSpacing[i] - spacing[i]) > 1e-6) + { + std::cerr << "Spacing mismatch in C locale at index " << i << std::endl; + std::cerr << "Expected: " << spacing[i] << ", Got: " << readSpacing[i] << std::endl; + return EXIT_FAILURE; + } + } + + // Verify origin + for (unsigned int i = 0; i < Dimension; ++i) + { + if (itk::Math::abs(readOrigin[i] - origin[i]) > 1e-6) + { + std::cerr << "Origin mismatch in C locale at index " << i << std::endl; + std::cerr << "Expected: " << origin[i] << ", Got: " << readOrigin[i] << std::endl; + return EXIT_FAILURE; + } + } + } + + // Test 2: Read with de_DE locale (the problematic case) + // Try to set German locale; if not available, skip this part + if (setlocale(LC_NUMERIC, "de_DE.UTF-8") != nullptr) + { + std::cout << "Testing with de_DE.UTF-8 locale..." << std::endl; + + using ReaderType = itk::ImageFileReader; + auto reader = ReaderType::New(); + reader->SetFileName(filename); + reader->SetImageIO(itk::NrrdImageIO::New()); + + ITK_TRY_EXPECT_NO_EXCEPTION(reader->Update()); + + ImageType::Pointer readImage = reader->GetOutput(); + ImageType::SpacingType readSpacing = readImage->GetSpacing(); + ImageType::PointType readOrigin = readImage->GetOrigin(); + + std::cout << "de_DE locale - Read spacing: " << readSpacing << std::endl; + std::cout << "de_DE locale - Read origin: " << readOrigin << std::endl; + + // Verify spacing - this is the critical test + for (unsigned int i = 0; i < Dimension; ++i) + { + if (itk::Math::abs(readSpacing[i] - spacing[i]) > 1e-6) + { + std::cerr << "Spacing mismatch in de_DE locale at index " << i << std::endl; + std::cerr << "Expected: " << spacing[i] << ", Got: " << readSpacing[i] << std::endl; + std::cerr << "This indicates locale-dependent parsing is still occurring!" << std::endl; + return EXIT_FAILURE; + } + } + + // Verify origin + for (unsigned int i = 0; i < Dimension; ++i) + { + if (itk::Math::abs(readOrigin[i] - origin[i]) > 1e-6) + { + std::cerr << "Origin mismatch in de_DE locale at index " << i << std::endl; + std::cerr << "Expected: " << origin[i] << ", Got: " << readOrigin[i] << std::endl; + std::cerr << "This indicates locale-dependent parsing is still occurring!" << std::endl; + return EXIT_FAILURE; + } + } + + // Restore C locale + setlocale(LC_NUMERIC, "C"); + } + else + { + std::cout << "de_DE.UTF-8 locale not available, skipping locale-specific test" << std::endl; + } + + std::cout << "Test finished successfully." << std::endl; + return EXIT_SUCCESS; +} From 622bcb65c226dce33b0c9e58bdb2351c10229ffd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Dec 2025 16:27:35 +0000 Subject: [PATCH 2/5] ENH: Add thread-safe NumericLocale utility class Create thread-safe locale handling in ITKCommon to replace non-thread-safe setlocale() calls. Update NRRD reader to use the new utility. Thread safety implementation: - Windows: Uses _configthreadlocale() for thread-local locale - POSIX (Linux/macOS): Uses newlocale()/uselocale() for thread-local locale - Fallback: Mutex-protected global locale as last resort This addresses thread-safety concerns raised in code review. --- .../Core/Common/include/itkNumericLocale.h | 111 +++++++++++++++ Modules/Core/Common/src/CMakeLists.txt | 1 + Modules/Core/Common/src/itkNumericLocale.cxx | 131 ++++++++++++++++++ Modules/IO/NRRD/src/itkNrrdImageIO.cxx | 53 +------ 4 files changed, 250 insertions(+), 46 deletions(-) create mode 100644 Modules/Core/Common/include/itkNumericLocale.h create mode 100644 Modules/Core/Common/src/itkNumericLocale.cxx diff --git a/Modules/Core/Common/include/itkNumericLocale.h b/Modules/Core/Common/include/itkNumericLocale.h new file mode 100644 index 00000000000..4ad7b982f79 --- /dev/null +++ b/Modules/Core/Common/include/itkNumericLocale.h @@ -0,0 +1,111 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ +#ifndef itkNumericLocale_h +#define itkNumericLocale_h + +#include "itkMacro.h" +#include "ITKCommonExport.h" + +#include +#include +#include + +// Platform-specific includes for thread-safe locale handling +#if defined(_WIN32) +# include +#elif defined(__APPLE__) || defined(__linux__) || defined(__unix__) +// POSIX systems provide newlocale/uselocale in locale.h (C11) or xlocale.h (older) +# if defined(__APPLE__) +# include +# elif defined(__GLIBC__) +// glibc provides locale_t in locale.h +# else +// Try xlocale.h for other systems, fall back to locale.h +# if __has_include() +# include +# endif +# endif +#endif + +namespace itk +{ + +/** \class NumericLocale + * \brief RAII class for thread-safe temporary setting of LC_NUMERIC locale to "C". + * + * This class provides a thread-safe mechanism to temporarily set the LC_NUMERIC + * locale to "C" for locale-independent parsing and formatting of floating-point + * numbers. The original locale is automatically restored when the object goes + * out of scope. + * + * This is particularly useful when parsing file formats that use dot as decimal + * separator (like NRRD, VTK, etc.) regardless of the system locale setting. + * + * Thread safety: + * - On POSIX systems (Linux, macOS, BSD): Uses thread-local locale via uselocale() + * - On Windows: Uses thread-local locale via _configthreadlocale() + * - Fallback: Uses global setlocale() with mutex protection (not fully thread-safe + * for concurrent I/O operations, but prevents corruption of locale state) + * + * Example usage: + * \code + * { + * NumericLocale cLocale; + * // Parse file with dot decimal separator + * double value = std::strtod("3.14159", nullptr); + * // Locale automatically restored here + * } + * \endcode + * + * \ingroup ITKCommon + */ +class ITKCommon_EXPORT NumericLocale +{ +public: + /** Constructor: Saves current LC_NUMERIC locale and sets it to "C" */ + NumericLocale(); + + /** Destructor: Restores the original LC_NUMERIC locale */ + ~NumericLocale(); + + // Delete copy and move operations + NumericLocale(const NumericLocale &) = delete; + NumericLocale & + operator=(const NumericLocale &) = delete; + NumericLocale(NumericLocale &&) = delete; + NumericLocale & + operator=(NumericLocale &&) = delete; + +private: +#if defined(_WIN32) + // Windows: store previous thread-specific locale setting + int m_PreviousThreadLocaleSetting{ -1 }; + char * m_SavedLocale{ nullptr }; +#elif defined(__APPLE__) || defined(__linux__) || defined(__unix__) + // POSIX: store previous thread-local locale + locale_t m_PreviousLocale{ nullptr }; + locale_t m_CLocale{ nullptr }; +#else + // Fallback: global locale (protected by mutex in implementation) + char * m_SavedLocale{ nullptr }; +#endif +}; + +} // end namespace itk + +#endif // itkNumericLocale_h diff --git a/Modules/Core/Common/src/CMakeLists.txt b/Modules/Core/Common/src/CMakeLists.txt index 0f66a451476..820958ec516 100644 --- a/Modules/Core/Common/src/CMakeLists.txt +++ b/Modules/Core/Common/src/CMakeLists.txt @@ -66,6 +66,7 @@ set( itkLogger.cxx itkLogOutput.cxx itkLoggerOutput.cxx + itkNumericLocale.cxx itkProgressAccumulator.cxx itkTotalProgressReporter.cxx itkNumericTraits.cxx diff --git a/Modules/Core/Common/src/itkNumericLocale.cxx b/Modules/Core/Common/src/itkNumericLocale.cxx new file mode 100644 index 00000000000..9c9477b5ee4 --- /dev/null +++ b/Modules/Core/Common/src/itkNumericLocale.cxx @@ -0,0 +1,131 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +#include "itkNumericLocale.h" + +#include + +namespace itk +{ + +#if defined(_WIN32) + +// Windows implementation using thread-specific locale +NumericLocale::NumericLocale() +{ + // Enable thread-specific locale for this thread + m_PreviousThreadLocaleSetting = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); + + // Save current LC_NUMERIC locale + const char * currentLocale = setlocale(LC_NUMERIC, nullptr); + if (currentLocale) + { + m_SavedLocale = _strdup(currentLocale); + } + + // Set to C locale for parsing + setlocale(LC_NUMERIC, "C"); +} + +NumericLocale::~NumericLocale() +{ + // Restore original locale + if (m_SavedLocale) + { + setlocale(LC_NUMERIC, m_SavedLocale); + free(m_SavedLocale); + } + + // Restore previous thread-specific locale setting + if (m_PreviousThreadLocaleSetting != -1) + { + _configthreadlocale(m_PreviousThreadLocaleSetting); + } +} + +#elif defined(__APPLE__) || defined(__linux__) || defined(__unix__) + +// POSIX implementation using thread-local locale (uselocale/newlocale) +NumericLocale::NumericLocale() +{ + // Create a new C locale + m_CLocale = newlocale(LC_NUMERIC_MASK, "C", nullptr); + + if (m_CLocale) + { + // Set the C locale for this thread and save the previous locale + m_PreviousLocale = uselocale(m_CLocale); + } +} + +NumericLocale::~NumericLocale() +{ + // Restore the previous locale for this thread + if (m_PreviousLocale) + { + uselocale(m_PreviousLocale); + } + + // Free the C locale + if (m_CLocale) + { + freelocale(m_CLocale); + } +} + +#else + +// Fallback implementation using global locale with mutex protection +// This is not ideal but provides some protection against concurrent access +namespace +{ +std::mutex localeMutex; +} + +NumericLocale::NumericLocale() +{ + // Lock mutex to protect global locale state + localeMutex.lock(); + + // Save current LC_NUMERIC locale + const char * currentLocale = setlocale(LC_NUMERIC, nullptr); + if (currentLocale) + { + m_SavedLocale = strdup(currentLocale); + } + + // Set to C locale for parsing + setlocale(LC_NUMERIC, "C"); +} + +NumericLocale::~NumericLocale() +{ + // Restore original locale + if (m_SavedLocale) + { + setlocale(LC_NUMERIC, m_SavedLocale); + free(m_SavedLocale); + } + + // Unlock mutex + localeMutex.unlock(); +} + +#endif + +} // end namespace itk diff --git a/Modules/IO/NRRD/src/itkNrrdImageIO.cxx b/Modules/IO/NRRD/src/itkNrrdImageIO.cxx index 8c000a7d948..c630507a020 100644 --- a/Modules/IO/NRRD/src/itkNrrdImageIO.cxx +++ b/Modules/IO/NRRD/src/itkNrrdImageIO.cxx @@ -22,54 +22,12 @@ #include "itkMetaDataObject.h" #include "itkIOCommon.h" #include "itkFloatingPointExceptions.h" +#include "itkNumericLocale.h" #include -#include -#include -#include namespace { -// RAII class to temporarily set LC_NUMERIC locale to "C" for locale-independent -// parsing of floating-point numbers in NRRD headers. -// This prevents issues in locales that use comma as decimal separator (e.g., de_DE.UTF-8). -class ScopedCNumericLocale -{ -public: - ScopedCNumericLocale() - { - // Save current LC_NUMERIC locale - const char * currentLocale = setlocale(LC_NUMERIC, nullptr); - if (currentLocale) - { - m_SavedLocale = strdup(currentLocale); - } - // Set to C locale for parsing - setlocale(LC_NUMERIC, "C"); - } - - ~ScopedCNumericLocale() - { - // Restore original locale - if (m_SavedLocale) - { - setlocale(LC_NUMERIC, m_SavedLocale); - free(m_SavedLocale); - } - } - - // Delete copy and move operations - ScopedCNumericLocale(const ScopedCNumericLocale &) = delete; - ScopedCNumericLocale & - operator=(const ScopedCNumericLocale &) = delete; - ScopedCNumericLocale(ScopedCNumericLocale &&) = delete; - ScopedCNumericLocale & - operator=(ScopedCNumericLocale &&) = delete; - -private: - char * m_SavedLocale{ nullptr }; -}; - // This function determines which NRRD axis should be used for pixel component in the ITK image, // and which NRRD axes should be used for the ITK image axes. // The pixel component axis is the first NRRD range axis (preferably a non-list axis). @@ -441,7 +399,8 @@ NrrdImageIO::ReadImageInformation() // Set LC_NUMERIC to "C" locale to ensure locale-independent parsing // of floating-point values in NRRD headers (spacing, origin, direction, etc.). // This prevents issues in locales that use comma as decimal separator. - ScopedCNumericLocale cLocale; + // Using thread-safe NumericLocale from ITKCommon. + NumericLocale cLocale; // this is the mechanism by which we tell nrrdLoad to read // just the header, and none of the data @@ -973,7 +932,8 @@ NrrdImageIO::Read(void * buffer) // Set LC_NUMERIC to "C" locale to ensure locale-independent parsing // of floating-point values in NRRD headers. - ScopedCNumericLocale cLocale; + // Using thread-safe NumericLocale from ITKCommon. + NumericLocale cLocale; // Read in the nrrd. Yes, this means that the header is being read // twice: once by NrrdImageIO::ReadImageInformation, and once here @@ -1392,7 +1352,8 @@ NrrdImageIO::Write(const void * buffer) // Set LC_NUMERIC to "C" locale to ensure locale-independent formatting // of floating-point values when writing NRRD headers. - ScopedCNumericLocale cLocale; + // Using thread-safe NumericLocale from ITKCommon. + NumericLocale cLocale; // Write the nrrd to file. if (nrrdSave(this->GetFileName(), nrrd, nio)) From 461a8a777a263e1216281e876643e88761e8056b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Dec 2025 19:29:49 +0000 Subject: [PATCH 3/5] Address code review comments on NumericLocale - Document strdup/newlocale failure handling behavior - Fix POSIX destructor to unconditionally restore locale (handles LC_GLOBAL_LOCALE) - Add noexcept to fallback destructor and document setlocale safety - Fix issue number placeholder in test (5683) - Clarify test comments about expected behavior without the fix These changes improve code robustness and documentation based on automated code review feedback. --- Modules/Core/Common/src/itkNumericLocale.cxx | 26 +++++++++++++------- Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx | 10 ++++---- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Modules/Core/Common/src/itkNumericLocale.cxx b/Modules/Core/Common/src/itkNumericLocale.cxx index 9c9477b5ee4..d5f989a030e 100644 --- a/Modules/Core/Common/src/itkNumericLocale.cxx +++ b/Modules/Core/Common/src/itkNumericLocale.cxx @@ -36,6 +36,8 @@ NumericLocale::NumericLocale() if (currentLocale) { m_SavedLocale = _strdup(currentLocale); + // If _strdup fails (returns nullptr), m_SavedLocale remains nullptr + // and the locale will not be restored in the destructor } // Set to C locale for parsing @@ -64,26 +66,27 @@ NumericLocale::~NumericLocale() NumericLocale::NumericLocale() { // Create a new C locale + // If newlocale fails (returns nullptr), m_CLocale remains nullptr + // and the locale will not be changed - this is a safe fallback m_CLocale = newlocale(LC_NUMERIC_MASK, "C", nullptr); if (m_CLocale) { // Set the C locale for this thread and save the previous locale + // uselocale returns the previous locale, which may be LC_GLOBAL_LOCALE m_PreviousLocale = uselocale(m_CLocale); } } NumericLocale::~NumericLocale() { - // Restore the previous locale for this thread - if (m_PreviousLocale) - { - uselocale(m_PreviousLocale); - } - - // Free the C locale + // If we created and installed a C locale for this thread, + // restore the previous locale and free the C locale. + // Always restore if m_CLocale is set, as m_PreviousLocale may be + // LC_GLOBAL_LOCALE which is a valid non-null value. if (m_CLocale) { + uselocale(m_PreviousLocale); freelocale(m_CLocale); } } @@ -100,6 +103,8 @@ std::mutex localeMutex; NumericLocale::NumericLocale() { // Lock mutex to protect global locale state + // Using direct lock/unlock instead of lock_guard to maintain compatibility + // with the RAII pattern where unlock happens in destructor localeMutex.lock(); // Save current LC_NUMERIC locale @@ -107,13 +112,16 @@ NumericLocale::NumericLocale() if (currentLocale) { m_SavedLocale = strdup(currentLocale); + // If strdup fails (returns nullptr), m_SavedLocale remains nullptr + // and the locale will not be restored in the destructor } // Set to C locale for parsing + // Note: setlocale is a C function and does not throw exceptions setlocale(LC_NUMERIC, "C"); } -NumericLocale::~NumericLocale() +NumericLocale::~NumericLocale() noexcept { // Restore original locale if (m_SavedLocale) @@ -122,7 +130,7 @@ NumericLocale::~NumericLocale() free(m_SavedLocale); } - // Unlock mutex + // Unlock mutex - must always happen to avoid deadlock localeMutex.unlock(); } diff --git a/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx b/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx index 8d7d304606d..a096903086d 100644 --- a/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx +++ b/Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx @@ -24,7 +24,7 @@ #include "itkTestingMacros.h" // Test that NRRD reader handles locale-dependent parsing correctly. -// This test verifies the fix for: https://github.com/InsightSoftwareConsortium/ITK/issues/XXXX +// This test verifies the fix for: https://github.com/InsightSoftwareConsortium/ITK/issues/5683 int itkNrrdLocaleTest(int argc, char * argv[]) @@ -55,11 +55,11 @@ itkNrrdLocaleTest(int argc, char * argv[]) image->FillBuffer(42.0f); // Set spacing with fractional values that would be misparsed - // in locales using comma as decimal separator + // in locales using comma as decimal separator (without the fix) ImageType::SpacingType spacing; - spacing[0] = 3.5; // Would be parsed correctly even with truncation - spacing[1] = 0.878906; // Would become 0.0 in de_DE locale, causing error - spacing[2] = 2.2; // Would be parsed incorrectly + spacing[0] = 3.5; // Without fix: would be truncated to 3 in de_DE locale + spacing[1] = 0.878906; // Without fix: would become 0.0 in de_DE locale, causing zero-spacing error + spacing[2] = 2.2; // Without fix: would be truncated to 2 in de_DE locale image->SetSpacing(spacing); From 24262e24bb32bc07faf1ff4c94930a4f62465a4b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 22 Dec 2025 17:23:41 +0000 Subject: [PATCH 4/5] Refactor NumericLocale per code review feedback - pImpl pattern removes platform headers from public API - CMake try_compile detects ITK_HAS_NEWLOCALE/ITK_HAS_CONFIGTHREADLOCALE - Fallback only warns if locale != C, doesn't modify - Removed mutex fallback that could serialize I/O --- .../CMake/itkCheckHasConfigthreadlocale.cxx | 31 ++++++ .../Common/CMake/itkCheckHasNewlocale.cxx | 37 ++++++ Modules/Core/Common/CMakeLists.txt | 22 ++++ .../Core/Common/include/itkNumericLocale.h | 45 ++------ .../Core/Common/src/itkConfigurePrivate.h.in | 5 + Modules/Core/Common/src/itkNumericLocale.cxx | 105 ++++++++++-------- 6 files changed, 162 insertions(+), 83 deletions(-) create mode 100644 Modules/Core/Common/CMake/itkCheckHasConfigthreadlocale.cxx create mode 100644 Modules/Core/Common/CMake/itkCheckHasNewlocale.cxx diff --git a/Modules/Core/Common/CMake/itkCheckHasConfigthreadlocale.cxx b/Modules/Core/Common/CMake/itkCheckHasConfigthreadlocale.cxx new file mode 100644 index 00000000000..959c7830b65 --- /dev/null +++ b/Modules/Core/Common/CMake/itkCheckHasConfigthreadlocale.cxx @@ -0,0 +1,31 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +// Check if _configthreadlocale is available (Windows thread-local locale) +#include + +int +main() +{ + int prev = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); + if (prev != -1) + { + _configthreadlocale(prev); + } + return 0; +} diff --git a/Modules/Core/Common/CMake/itkCheckHasNewlocale.cxx b/Modules/Core/Common/CMake/itkCheckHasNewlocale.cxx new file mode 100644 index 00000000000..674f962e1ce --- /dev/null +++ b/Modules/Core/Common/CMake/itkCheckHasNewlocale.cxx @@ -0,0 +1,37 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +// Check if newlocale and uselocale are available (POSIX thread-local locale) +#include + +#if defined(__APPLE__) +# include +#endif + +int +main() +{ + locale_t loc = newlocale(LC_NUMERIC_MASK, "C", nullptr); + if (loc) + { + locale_t prev = uselocale(loc); + uselocale(prev); + freelocale(loc); + } + return 0; +} diff --git a/Modules/Core/Common/CMakeLists.txt b/Modules/Core/Common/CMakeLists.txt index 564cccbede0..9ba04003965 100644 --- a/Modules/Core/Common/CMakeLists.txt +++ b/Modules/Core/Common/CMakeLists.txt @@ -157,6 +157,28 @@ try_compile( ${CMAKE_CURRENT_SOURCE_DIR}/CMake/itkCheckHasSchedGetAffinity.cxx ) +# Check for thread-local locale functions for NumericLocale +try_compile( + ITK_HAS_NEWLOCALE + ${ITK_BINARY_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/CMake/itkCheckHasNewlocale.cxx +) + +try_compile( + ITK_HAS_CONFIGTHREADLOCALE + ${ITK_BINARY_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/CMake/itkCheckHasConfigthreadlocale.cxx +) + +# Warn if no thread-safe locale method is available +if(NOT ITK_HAS_NEWLOCALE AND NOT ITK_HAS_CONFIGTHREADLOCALE) + message(WARNING + "Neither POSIX newlocale/uselocale nor Windows _configthreadlocale detected. " + "Thread-safe numeric locale handling is not available. " + "Locale must be managed at the application level if needed." + ) +endif() + #----------------------------------------------------------------------------- # Make default visibility as an option for generated export header diff --git a/Modules/Core/Common/include/itkNumericLocale.h b/Modules/Core/Common/include/itkNumericLocale.h index 4ad7b982f79..767d88a8612 100644 --- a/Modules/Core/Common/include/itkNumericLocale.h +++ b/Modules/Core/Common/include/itkNumericLocale.h @@ -21,26 +21,7 @@ #include "itkMacro.h" #include "ITKCommonExport.h" -#include -#include -#include - -// Platform-specific includes for thread-safe locale handling -#if defined(_WIN32) -# include -#elif defined(__APPLE__) || defined(__linux__) || defined(__unix__) -// POSIX systems provide newlocale/uselocale in locale.h (C11) or xlocale.h (older) -# if defined(__APPLE__) -# include -# elif defined(__GLIBC__) -// glibc provides locale_t in locale.h -# else -// Try xlocale.h for other systems, fall back to locale.h -# if __has_include() -# include -# endif -# endif -#endif +#include namespace itk { @@ -57,10 +38,10 @@ namespace itk * separator (like NRRD, VTK, etc.) regardless of the system locale setting. * * Thread safety: - * - On POSIX systems (Linux, macOS, BSD): Uses thread-local locale via uselocale() - * - On Windows: Uses thread-local locale via _configthreadlocale() - * - Fallback: Uses global setlocale() with mutex protection (not fully thread-safe - * for concurrent I/O operations, but prevents corruption of locale state) + * - On POSIX systems (when newlocale/uselocale are available): Uses thread-local locale + * - On Windows (when _configthreadlocale is available): Uses thread-specific locale + * - Fallback (when neither is available): Issues a warning if locale differs from "C", + * but does not change the locale. Applications must manage locale externally. * * Example usage: * \code @@ -92,18 +73,10 @@ class ITKCommon_EXPORT NumericLocale operator=(NumericLocale &&) = delete; private: -#if defined(_WIN32) - // Windows: store previous thread-specific locale setting - int m_PreviousThreadLocaleSetting{ -1 }; - char * m_SavedLocale{ nullptr }; -#elif defined(__APPLE__) || defined(__linux__) || defined(__unix__) - // POSIX: store previous thread-local locale - locale_t m_PreviousLocale{ nullptr }; - locale_t m_CLocale{ nullptr }; -#else - // Fallback: global locale (protected by mutex in implementation) - char * m_SavedLocale{ nullptr }; -#endif + // Forward declaration of implementation structure + struct Impl; + // Pointer to implementation (pImpl idiom) + std::unique_ptr m_Impl; }; } // end namespace itk diff --git a/Modules/Core/Common/src/itkConfigurePrivate.h.in b/Modules/Core/Common/src/itkConfigurePrivate.h.in index 1a19605a39d..796c21dea6c 100644 --- a/Modules/Core/Common/src/itkConfigurePrivate.h.in +++ b/Modules/Core/Common/src/itkConfigurePrivate.h.in @@ -33,4 +33,9 @@ // defined if fenv_t struct as __cw member #cmakedefine ITK_HAS_STRUCT_FENV_T_CW +// defined if newlocale/uselocale are available (POSIX thread-local locale) +#cmakedefine ITK_HAS_NEWLOCALE +// defined if _configthreadlocale is available (Windows thread-local locale) +#cmakedefine ITK_HAS_CONFIGTHREADLOCALE + #endif //itkConfigurePrivate_h diff --git a/Modules/Core/Common/src/itkNumericLocale.cxx b/Modules/Core/Common/src/itkNumericLocale.cxx index d5f989a030e..8ccd66767aa 100644 --- a/Modules/Core/Common/src/itkNumericLocale.cxx +++ b/Modules/Core/Common/src/itkNumericLocale.cxx @@ -17,25 +17,53 @@ *=========================================================================*/ #include "itkNumericLocale.h" +#include "itkConfigurePrivate.h" -#include +#include +#include +#include + +// Include platform-specific headers based on detected features +#ifdef ITK_HAS_NEWLOCALE +# if defined(__APPLE__) +# include +# endif +#endif namespace itk { -#if defined(_WIN32) +// Implementation structure definition +struct NumericLocale::Impl +{ +#ifdef ITK_HAS_CONFIGTHREADLOCALE + // Windows: thread-specific locale + int m_PreviousThreadLocaleSetting{ -1 }; + char * m_SavedLocale{ nullptr }; +#elif defined(ITK_HAS_NEWLOCALE) + // POSIX: thread-local locale + locale_t m_PreviousLocale{ nullptr }; + locale_t m_CLocale{ nullptr }; +#else + // Fallback: no locale change, only check and warn + bool m_WarningIssued{ false }; +#endif +}; + +#ifdef ITK_HAS_CONFIGTHREADLOCALE // Windows implementation using thread-specific locale NumericLocale::NumericLocale() + : m_Impl(new Impl()) { // Enable thread-specific locale for this thread - m_PreviousThreadLocaleSetting = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); + m_Impl->m_PreviousThreadLocaleSetting = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); // Save current LC_NUMERIC locale const char * currentLocale = setlocale(LC_NUMERIC, nullptr); if (currentLocale) { - m_SavedLocale = _strdup(currentLocale); + m_Impl->m_SavedLocale = _strdup(currentLocale); // If _strdup fails (returns nullptr), m_SavedLocale remains nullptr // and the locale will not be restored in the destructor } @@ -47,34 +75,35 @@ NumericLocale::NumericLocale() NumericLocale::~NumericLocale() { // Restore original locale - if (m_SavedLocale) + if (m_Impl->m_SavedLocale) { - setlocale(LC_NUMERIC, m_SavedLocale); - free(m_SavedLocale); + setlocale(LC_NUMERIC, m_Impl->m_SavedLocale); + free(m_Impl->m_SavedLocale); } // Restore previous thread-specific locale setting - if (m_PreviousThreadLocaleSetting != -1) + if (m_Impl->m_PreviousThreadLocaleSetting != -1) { - _configthreadlocale(m_PreviousThreadLocaleSetting); + _configthreadlocale(m_Impl->m_PreviousThreadLocaleSetting); } } -#elif defined(__APPLE__) || defined(__linux__) || defined(__unix__) +#elif defined(ITK_HAS_NEWLOCALE) // POSIX implementation using thread-local locale (uselocale/newlocale) NumericLocale::NumericLocale() + : m_Impl(new Impl()) { // Create a new C locale // If newlocale fails (returns nullptr), m_CLocale remains nullptr // and the locale will not be changed - this is a safe fallback - m_CLocale = newlocale(LC_NUMERIC_MASK, "C", nullptr); + m_Impl->m_CLocale = newlocale(LC_NUMERIC_MASK, "C", nullptr); - if (m_CLocale) + if (m_Impl->m_CLocale) { // Set the C locale for this thread and save the previous locale // uselocale returns the previous locale, which may be LC_GLOBAL_LOCALE - m_PreviousLocale = uselocale(m_CLocale); + m_Impl->m_PreviousLocale = uselocale(m_Impl->m_CLocale); } } @@ -84,54 +113,36 @@ NumericLocale::~NumericLocale() // restore the previous locale and free the C locale. // Always restore if m_CLocale is set, as m_PreviousLocale may be // LC_GLOBAL_LOCALE which is a valid non-null value. - if (m_CLocale) + if (m_Impl->m_CLocale) { - uselocale(m_PreviousLocale); - freelocale(m_CLocale); + uselocale(m_Impl->m_PreviousLocale); + freelocale(m_Impl->m_CLocale); } } #else -// Fallback implementation using global locale with mutex protection -// This is not ideal but provides some protection against concurrent access -namespace -{ -std::mutex localeMutex; -} - +// Fallback implementation - only check locale and issue warning if not "C" +// Do not modify the locale, application must manage it externally NumericLocale::NumericLocale() + : m_Impl(new Impl()) { - // Lock mutex to protect global locale state - // Using direct lock/unlock instead of lock_guard to maintain compatibility - // with the RAII pattern where unlock happens in destructor - localeMutex.lock(); - - // Save current LC_NUMERIC locale + // Check if current locale is compatible with expected "C" locale const char * currentLocale = setlocale(LC_NUMERIC, nullptr); - if (currentLocale) + if (currentLocale && std::strcmp(currentLocale, "C") != 0) { - m_SavedLocale = strdup(currentLocale); - // If strdup fails (returns nullptr), m_SavedLocale remains nullptr - // and the locale will not be restored in the destructor + // Issue warning only once per instance + itkWarningMacro("LC_NUMERIC locale is '" << currentLocale << "' (not 'C'). " + << "Thread-safe locale functions not available. " + << "Locale-dependent number parsing may cause issues. " + << "Please manage locale at application level."); + m_Impl->m_WarningIssued = true; } - - // Set to C locale for parsing - // Note: setlocale is a C function and does not throw exceptions - setlocale(LC_NUMERIC, "C"); } -NumericLocale::~NumericLocale() noexcept +NumericLocale::~NumericLocale() { - // Restore original locale - if (m_SavedLocale) - { - setlocale(LC_NUMERIC, m_SavedLocale); - free(m_SavedLocale); - } - - // Unlock mutex - must always happen to avoid deadlock - localeMutex.unlock(); + // No action needed in fallback - we don't change the locale } #endif From 0fc849d626f132ab1693f74b3de271da2d759f19 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 23 Dec 2025 16:13:32 +0000 Subject: [PATCH 5/5] Add Google Test for NumericLocale class Add comprehensive Google Test suite for itk::NumericLocale: - Test temporary setting to C locale - Test floating-point parsing with dot decimal separator - Test nesting behavior - Test with different initial locale (de_DE.UTF-8) - Test sequential uses - Test basic RAII behavior All tests pass successfully. --- Modules/Core/Common/test/CMakeLists.txt | 1 + .../Common/test/itkNumericLocaleGTest.cxx | 158 ++++++++++++++++++ 2 files changed, 159 insertions(+) create mode 100644 Modules/Core/Common/test/itkNumericLocaleGTest.cxx diff --git a/Modules/Core/Common/test/CMakeLists.txt b/Modules/Core/Common/test/CMakeLists.txt index d89ec1e6e8c..0e030722b5d 100644 --- a/Modules/Core/Common/test/CMakeLists.txt +++ b/Modules/Core/Common/test/CMakeLists.txt @@ -1890,6 +1890,7 @@ set( itkMersenneTwisterRandomVariateGeneratorGTest.cxx itkNeighborhoodAllocatorGTest.cxx itkNumberToStringGTest.cxx + itkNumericLocaleGTest.cxx itkObjectFactoryBaseGTest.cxx itkOffsetGTest.cxx itkOptimizerParametersGTest.cxx diff --git a/Modules/Core/Common/test/itkNumericLocaleGTest.cxx b/Modules/Core/Common/test/itkNumericLocaleGTest.cxx new file mode 100644 index 00000000000..99e6879ece8 --- /dev/null +++ b/Modules/Core/Common/test/itkNumericLocaleGTest.cxx @@ -0,0 +1,158 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +#include "itkNumericLocale.h" +#include +#include +#include +#include + +// Test that NumericLocale temporarily sets LC_NUMERIC to "C" +TEST(NumericLocale, TemporarilySetsToCLocale) +{ + // Save initial locale + const char * initialLocale = setlocale(LC_NUMERIC, nullptr); + ASSERT_NE(initialLocale, nullptr); + std::string savedInitialLocale(initialLocale); + + { + // Create NumericLocale - should set to "C" + itk::NumericLocale numericLocale; + + const char * currentLocale = setlocale(LC_NUMERIC, nullptr); + ASSERT_NE(currentLocale, nullptr); + EXPECT_STREQ(currentLocale, "C"); + } + + // After NumericLocale destroyed, locale should be restored + const char * restoredLocale = setlocale(LC_NUMERIC, nullptr); + ASSERT_NE(restoredLocale, nullptr); + EXPECT_STREQ(restoredLocale, savedInitialLocale.c_str()); +} + +// Test that numeric parsing works correctly with NumericLocale +TEST(NumericLocale, ParsesFloatsWithDotDecimalSeparator) +{ + { + itk::NumericLocale numericLocale; + + // Parse floating-point number with dot as decimal separator + double value = std::strtod("3.14159", nullptr); + EXPECT_DOUBLE_EQ(value, 3.14159); + + value = std::strtod("0.878906", nullptr); + EXPECT_DOUBLE_EQ(value, 0.878906); + + value = std::strtod("2.5", nullptr); + EXPECT_DOUBLE_EQ(value, 2.5); + } +} + +// Test that NumericLocale can be nested +TEST(NumericLocale, SupportsNesting) +{ + const char * initialLocale = setlocale(LC_NUMERIC, nullptr); + ASSERT_NE(initialLocale, nullptr); + std::string savedInitialLocale(initialLocale); + + { + itk::NumericLocale outerLocale; + EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), "C"); + + { + itk::NumericLocale innerLocale; + EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), "C"); + } + + // After inner destroyed, still in C locale + EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), "C"); + } + + // After both destroyed, locale is restored + EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), savedInitialLocale.c_str()); +} + +// Test with a different locale if available (optional test) +TEST(NumericLocale, WorksWithDifferentInitialLocale) +{ + // Try to set a locale with comma as decimal separator + const char * germanLocale = setlocale(LC_NUMERIC, "de_DE.UTF-8"); + + if (germanLocale != nullptr) + { + // Verify we're in German locale (comma separator) + const char * currentLocale = setlocale(LC_NUMERIC, nullptr); + ASSERT_NE(currentLocale, nullptr); + std::string savedLocale(currentLocale); + + // Without NumericLocale, parsing with dot would fail in German locale + // (This test verifies the problem we're fixing) + double valueWithoutFix = std::strtod("0.878906", nullptr); + // In de_DE locale, this would parse as 0.0 (stops at dot) + EXPECT_EQ(valueWithoutFix, 0.0); + + { + // With NumericLocale, parsing should work correctly + itk::NumericLocale numericLocale; + + double valueWithFix = std::strtod("0.878906", nullptr); + EXPECT_DOUBLE_EQ(valueWithFix, 0.878906); + + double value2 = std::strtod("3.5", nullptr); + EXPECT_DOUBLE_EQ(value2, 3.5); + } + + // After NumericLocale destroyed, we should be back in German locale + EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), savedLocale.c_str()); + + // Restore to C locale for other tests + setlocale(LC_NUMERIC, "C"); + } + else + { + // de_DE.UTF-8 locale not available, skip this test + GTEST_SKIP() << "de_DE.UTF-8 locale not available on this system"; + } +} + +// Test that multiple sequential uses work correctly +TEST(NumericLocale, SupportsSequentialUses) +{ + for (int i = 0; i < 3; ++i) + { + itk::NumericLocale numericLocale; + double value = std::strtod("1.5", nullptr); + EXPECT_DOUBLE_EQ(value, 1.5); + } +} + +// Test basic RAII behavior +TEST(NumericLocale, BasicRAII) +{ + const char * initialLocale = setlocale(LC_NUMERIC, nullptr); + ASSERT_NE(initialLocale, nullptr); + std::string savedInitialLocale(initialLocale); + + // Create and immediately destroy + { + itk::NumericLocale temp; + } + + // Locale should be restored + EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), savedInitialLocale.c_str()); +}