Skip to content

Commit 2aa5799

Browse files
committed
BUG: Locale-independent DICOM DS parsing in DCMTKTransformIO
DCMTKTransformIO::Read() parsed DICOM Decimal-String matrix values with std::stod inside a NumericLocale scope. On macOS 13 and earlier libc the non-_l std::stod ignores the thread-local locale NumericLocale installs via uselocale(), so transform matrices parsed wrong under a decimal-comma locale -- the same defect as the NRRD reader. Add NumericLocale::StringToDouble (strtod_l against a cached "C" locale, thread-safe via a magic static) and use it at the two DS parse sites, dropping the now-redundant RAII guard. Replace the WorksWithDifferentInitialLocale test -- which asserted the RAII + plain std::strtod behavior that macOS 13 libc cannot provide -- with StringToDoubleIsLocaleIndependent, exercising the robust helper.
1 parent 7d8a1bb commit 2aa5799

4 files changed

Lines changed: 49 additions & 60 deletions

File tree

Modules/Core/Common/include/itkNumericLocale.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ class ITKCommon_EXPORT NumericLocale
6464
/** Destructor: Restores the original LC_NUMERIC locale */
6565
~NumericLocale();
6666

67+
/** Parse a decimal string with an explicit "C" locale on every platform.
68+
* The RAII scope only switches the thread-local locale, which non-_l
69+
* std::strtod ignores on macOS 13 and earlier libc; this uses strtod_l. */
70+
static double
71+
StringToDouble(const char * str);
72+
6773
// Delete copy and move operations
6874
NumericLocale(const NumericLocale &) = delete;
6975
NumericLocale &

Modules/Core/Common/src/itkNumericLocale.cxx

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,23 @@
3434
namespace itk
3535
{
3636

37+
// Parse with an explicit "C" locale so the result is locale-independent even
38+
// where non-_l std::strtod ignores the thread-local locale (macOS 13 libc).
39+
// The function-local static is thread-safe by C++11 magic-static semantics.
40+
double
41+
NumericLocale::StringToDouble(const char * str)
42+
{
43+
#if defined(ITK_HAS_NEWLOCALE)
44+
static const locale_t cLocale = newlocale(LC_NUMERIC_MASK, "C", static_cast<locale_t>(nullptr));
45+
return cLocale ? strtod_l(str, nullptr, cLocale) : std::strtod(str, nullptr);
46+
#elif defined(ITK_HAS_CONFIGTHREADLOCALE)
47+
static const _locale_t cLocale = _create_locale(LC_NUMERIC, "C");
48+
return cLocale ? _strtod_l(str, nullptr, cLocale) : std::strtod(str, nullptr);
49+
#else
50+
return std::strtod(str, nullptr);
51+
#endif
52+
}
53+
3754
// Implementation structure definition
3855
struct NumericLocale::Impl
3956
{

Modules/Core/Common/test/itkNumericLocaleGTest.cxx

Lines changed: 24 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -87,60 +87,6 @@ TEST(NumericLocale, SupportsNesting)
8787
EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), savedInitialLocale.c_str());
8888
}
8989

90-
// Test with a different locale if available (optional test)
91-
TEST(NumericLocale, WorksWithDifferentInitialLocale)
92-
{
93-
// Try to set a locale with comma as decimal separator
94-
const char * decimalCommaLocales[] = { "de_DE.UTF-8", "bs_BA.UTF-8", "bs-Latn-BA.UTF-8", "fr_FR.UTF-8" };
95-
96-
unsigned countAvailableLocales = 0;
97-
for (const char * locale : decimalCommaLocales)
98-
{
99-
const char * decimalCommaLocale = setlocale(LC_NUMERIC, locale);
100-
if (decimalCommaLocale != nullptr)
101-
{
102-
// Verify we're in German locale (comma separator)
103-
const char * currentLocale = setlocale(LC_NUMERIC, nullptr);
104-
ASSERT_NE(currentLocale, nullptr);
105-
std::string savedLocale(currentLocale);
106-
107-
// Without NumericLocale, parsing with dot would fail in German locale
108-
// (This test verifies the problem we're fixing)
109-
double valueWithoutFix = std::strtod("0.878906", nullptr);
110-
// In de_DE locale, this would parse as 0.0 (stops at dot)
111-
EXPECT_EQ(valueWithoutFix, 0.0);
112-
113-
{
114-
// With NumericLocale, parsing should work correctly
115-
itk::NumericLocale numericLocale;
116-
117-
double valueWithFix = std::strtod("0.878906", nullptr);
118-
EXPECT_DOUBLE_EQ(valueWithFix, 0.878906);
119-
120-
double value2 = std::strtod("3.5", nullptr);
121-
EXPECT_DOUBLE_EQ(value2, 3.5);
122-
}
123-
124-
// After NumericLocale destroyed, we should be back in German locale
125-
EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), savedLocale.c_str());
126-
127-
// Restore to C locale for other tests
128-
setlocale(LC_NUMERIC, "C");
129-
++countAvailableLocales;
130-
}
131-
}
132-
if (countAvailableLocales == 0)
133-
{
134-
GTEST_SKIP() << "Decimal-comma locales not available on this system. Locales tried: "
135-
<< sizeof(decimalCommaLocales) / sizeof(char *)
136-
<< ". Consider installing locales by\nsudo locale-gen de_DE.UTF-8";
137-
}
138-
else
139-
{
140-
std::cout << "Passed. Tested with " << countAvailableLocales << " decimal-comma locales." << std::endl;
141-
}
142-
}
143-
14490
// Test that multiple sequential uses work correctly
14591
TEST(NumericLocale, SupportsSequentialUses)
14692
{
@@ -167,3 +113,27 @@ TEST(NumericLocale, BasicRAII)
167113
// Locale should be restored
168114
EXPECT_STREQ(setlocale(LC_NUMERIC, nullptr), savedInitialLocale.c_str());
169115
}
116+
117+
// StringToDouble must parse '.' decimals regardless of the ambient locale,
118+
// even where non-_l std::strtod ignores uselocale() (macOS 13 libc).
119+
TEST(NumericLocale, StringToDoubleIsLocaleIndependent)
120+
{
121+
const char * commaLocales[] = { "de_DE.UTF-8", "fr_FR.UTF-8", "nl_NL.UTF-8", "it_IT.UTF-8" };
122+
unsigned int tested = 0;
123+
for (const char * locale : commaLocales)
124+
{
125+
if (setlocale(LC_NUMERIC, locale) == nullptr)
126+
{
127+
continue;
128+
}
129+
++tested;
130+
EXPECT_NEAR(itk::NumericLocale::StringToDouble("0.878906"), 0.878906, 1e-9);
131+
EXPECT_NEAR(itk::NumericLocale::StringToDouble("3.5"), 3.5, 1e-9);
132+
}
133+
setlocale(LC_NUMERIC, "C");
134+
EXPECT_DOUBLE_EQ(itk::NumericLocale::StringToDouble("2.25"), 2.25);
135+
if (tested == 0)
136+
{
137+
GTEST_SKIP() << "No decimal-comma locale installed to exercise the ambient-locale case.";
138+
}
139+
}

Modules/IO/IOTransformDCMTK/src/itkDCMTKTransformIO.hxx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,6 @@ template <typename TInternalComputationValueType>
9090
void
9191
DCMTKTransformIO<TInternalComputationValueType>::Read()
9292
{
93-
// DICOM Decimal-String values use '.' as decimal separator regardless
94-
// of system locale; force LC_NUMERIC=C for std::stod parsing below.
95-
const NumericLocale cLocale;
96-
9793
TransformListType & transformList = this->GetReadTransformList();
9894
transformList.clear();
9995

@@ -215,7 +211,7 @@ DCMTKTransformIO<TInternalComputationValueType>::Read()
215211
itkExceptionMacro("Could not get expected matrix entry.");
216212
}
217213
++matrixEntryIndex;
218-
transformParameters[row * Dimension + col] = std::stod(matrixString.c_str());
214+
transformParameters[row * Dimension + col] = NumericLocale::StringToDouble(matrixString.c_str());
219215
}
220216
result = currentMatrixSequenceItem->findAndGetOFString(
221217
DCM_FrameOfReferenceTransformationMatrix, matrixString, matrixEntryIndex);
@@ -224,7 +220,7 @@ DCMTKTransformIO<TInternalComputationValueType>::Read()
224220
itkExceptionMacro("Could not get expected matrix entry.");
225221
}
226222
++matrixEntryIndex;
227-
transformParameters[Dimension * Dimension + row] = std::stod(matrixString.c_str());
223+
transformParameters[Dimension * Dimension + row] = NumericLocale::StringToDouble(matrixString.c_str());
228224
}
229225
affineTransform->SetParameters(transformParameters);
230226

0 commit comments

Comments
 (0)