Hi Eli, > Can you explain the rationale for moving code > between gl_locale_name_thread and gl_locale_name_posix.
gl_locale_name() is defined, basically (forgive the shorthand notation) as gl_locale_name_thread() || gl_locale_name_posix() || gl_locale_name_default(). The semantics of your patch was to add specific code for native Windows: gl_locale_name_thread() || gl_locale_name_windows() || gl_locale_name_posix() || gl_locale_name_default(). There are three ways to actually implement this: (1) Define a function gl_locale_name_windows, explicitly. (2) Move gl_locale_name_windows() into gl_locale_name_thread(). (3) Move gl_locale_name_windows() into gl_locale_name_posix(). All three options produce the same value for gl_locale_name(), which is the only function that is used outside gnulib. But the test suite tests gl_locale_name_thread and gl_locale_name_posix individually. Pro and cons of (1),(2),(3): Pro (1): Very explicit. Cons (1): Code for a specific platform becomes mentioned in the .h file. Cons (2): gl_locale_name_windows() has nothing to do with threads. Cons (2): Requires some #if in the test suite, around line 480. Pro (3): Both gl_locale_name_windows() and gl_locale_name_posix() are related to setlocale(). Summary of scores: (1): 0 (2): -2 (3): +1 So, option (3) is the best one, for my taste. > I also don't understand why having a thread-level > code inside a function whose name ends with "_thread" makes less sense AFAICS, the body of gl_locale_name_windows() is not "thread-level code". It produces a value that depends on the return value of setlocale(), i.e. depends on global state of the libc, not on anything particular to the thread. > And how all that is related to the failures which are the trigger for > your proposal? The test failures occur because the distinction between what happens in gl_locale_name_thread and gl_locale_name_posix is visible to the test suite. Bruno
