[systemd-devel] [PATCH] __thread --> thread_local for C11 compat

[Shawn Landden]

On Sun, Dec 15, 2013 at 3:16 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Sun, 15.12.13 14:56, Shawn Landden (shawn at churchofgit.com) wrote:
>
>> On Sun, Dec 15, 2013 at 1:56 PM, Lennart Poettering
>> <lennart at poettering.net> wrote:
>> > On Sun, 15.12.13 13:19, Shawn Landden (shawn at churchofgit.com) wrote:
>> >
>> >> also make thread_local available w/o including <threads.h>
>> >
>> > Hmm, that looks a bit too early, no? Does gcc even support this? glibc
>> gcc-4.9 supports _Thread_local
>> http://gcc.gnu.org/gcc-4.9/changes.html
>
> Ah, nice. I figure that should be good enough then. (That said, my
> Fedora 20 doesn't have gcc 4.9 yet, so I can't test this...)
debian has gcc-snapshot in sid
>
>> switching -std=gnu99 to -std=gnu11 in Makefile I can compile
>> systemd w/o the clause that defines _Thread_local as __thread
>> and all tests pass.
>>
>> Perhaps we should add
>> || (__GNUC__ == 4 && __GNUC_MINOR__ >= 9)
>> to the check
>
> Hmm, checks for gcc versions are really only the last resorts. If there
> are feature test macros, we should use those, and should they break we
> an still resort to version checks.
>
>> >> +#if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__)
>> >> +#else
>> >> +#  define _Thread_local __thread
>
> I don't really like the "inverted" if thing with nothing on the true
> branch and only a false branch I must say.
>
> Also, should we really take the deviation of filling in the c
> low-levelism "_Thread_local" here? I think I'd prefer this:
agreed
>
> #ifndef thread_local
> #if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__)
> #define thread_local _Thread_local
> #else
> #define thread_local __thread
> #endif
>
> With that in place we stay away from defining our own low-level
> _Thread_local, instead we only define the high-level thread_local, and
> do so either to the C11 language feature or the gcc extension...
>
> If that looks OK (and is tested!) I'd be happy to merge such a patch.
>
to actually use this we would have to add this patch:

diff --git a/configure.ac b/configure.ac
index f85e86e..5ead127 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,6 +113,7 @@ AS_IF([test "x$enable_address_sanitizer" = "xyes"], [
       ])

 CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\
+        -std=gnu11 \
         -pipe \
         -Wall \
         -Wextra \