json-c fix, question about -D_REENTRANT

Hello,

The attached patch fixes the build of json-c when thread support is not available. This library does not require thread support: it was compatible with -D_REENTRANT in all cases, which is only possible when thread support is available.

However, I didn't commit this directly, because I am not sure what the right fix is. My fix is to just get rid of -D_REENTRANT.

However, according to
http://pauillac.inria.fr/~xleroy/linuxthreads/faq.html#H, all code used in a multithreaded application should be built with -D_REENTRANT, even if this code itself is not creating/manipulating threads.

So, technically, removing -D_REENTRANT seems like an incorrect: it should be kept if the toolchain supports thread.

However, looking quickly at a number of other libraries, I don't see any of them being careful about this. I know the -pthread gcc options passes both the -lpthread linker option and the -D_REENTRANT compiler option, but I don't see -pthread being used in libraries that don't need thread support, but that may be used in a multithreaded application.

Am I missing something?

Thanks,

Thomas

Fixes
http://autobuild.buildroot.org/results/1d9a2e44da3aa627265b03763ca324c609c0c61c/build-end.log.

---
 package/json-c/json-c-no-reentrant.patch |   33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 package/json-c/json-c-no-reentrant.patch

new file mode 100644
index 0000000..a01181c
@@ -0,0 +1,33 @@
+Do not pass -D_REENTRANT
+
+This flag is not needed to build this library, and prevents to build
+it with toolchains that don't have thread support.
+
+Since the Makefile.in change is a one-liner, we also do it in the
+patch to avoid having to autoreconfigure the package.
+
+Signed-off-by: Thomas Petazzoni <[email protected]>
+
+Index: b/Makefile.am
+===================================================================
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -1,4 +1,4 @@
+-AM_CFLAGS = -Wall -Wwrite-strings -Werror -std=gnu99 -D_GNU_SOURCE -D_REENTRANT
++AM_CFLAGS = -Wall -Wwrite-strings -Werror -std=gnu99 -D_GNU_SOURCE
+ 
+ EXTRA_DIST = README.html README-WIN32.html config.h.win32 doc
+ 
+Index: b/Makefile.in
+===================================================================
+--- a/Makefile.in
++++ b/Makefile.in
+@@ -219,7 +219,7 @@
+ top_build_prefix = @top_build_prefix@
+ top_builddir = @top_builddir@
+ top_srcdir = @top_srcdir@
+-AM_CFLAGS = -Wall -Wwrite-strings -Werror -std=gnu99 -D_GNU_SOURCE -D_REENTRANT
++AM_CFLAGS = -Wall -Wwrite-strings -Werror -std=gnu99 -D_GNU_SOURCE
+ EXTRA_DIST = README.html README-WIN32.html config.h.win32 doc
+ lib_LTLIBRARIES = libjson.la
+ pkgconfigdir = $(libdir)/pkgconfig

1.7.9.5

Thomas> Fixes
Thomas> http://autobuild.buildroot.org/results/1d9a2e44da3aa627265b03763ca324c609c0c61c/build-end.log.

Committed, thanks.

Bye, Peter Korsgaard

Note that that FAQ is pretty old.

I looked at the source of uClibc 0.32 and 0.33, and it looks like they
automatically select the reentrant versions of errno and getc if HAS_THREADS is defined. As to the _r reentrant functions, they're always there (except, for some reason, getlogin_r) - but for those you'd anyway get a compile or link error.

I expect glibc will always build the reentrant versions.

So I guess you can remove the -D_REENTRANT.

Regards,
Arnout

Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F