On 24/06/18 07:24, Bruno Haible wrote: > Pádraig Brady wrote: >> I'm going to apply my whar-single module to gnulib to tweak >> it so the main bottleneck of calling locale_charset repeatedly >> is removed from wcwidth() and mbrtowc(), in a simple manner, >> without the need for another API. > > This patch has two problems: > > 1) It introduces a test suite failure: A gnulib testdir > ./gnulib-tool --create-testdir --dir=... --single-configure mbrtowc > wcwidth wchar-single > fails the test-wcwidth test (test-wcwidth.c:59) on Mac OS X. > > 2) It introduces a multithread-safety bug: You added 'static' variables > also to the normal (non-singlethreaded) code path. Now, when you have > two threads which operate in different locales (via uselocale()) and > 1. thread1 does encoding = locale_charset (); > 2. thread2 does encoding = locale_charset (); > 3. thread1 does utf8_charset = STREQ_OPT (encoding, ...); > you've got a bug. > To eliminate such kinds of bugs, it is better to structure the code as > follows: > - A function that returns that value that can be cached. > - A function that does the caching AND NOTHING ELSE. So that it can be > #ifdefed > out easily. > - Code that uses the caching function. Do not add complexity here!
Excellent. Thanks for pushing those fixes!
