diff mbox

[RFC] Deprecate _REENTRANT and _THREAD_SAFE.

Message ID 611abaa8-6ffe-50b9-d369-46f8161b2529@panix.com
State New
Headers show

Commit Message

Zack Weinberg Dec. 6, 2016, 4:43 p.m. UTC
On 12/06/2016 10:36 AM, Florian Weimer wrote:
> On 12/06/2016 04:22 PM, Zack Weinberg wrote:

>> For many years, the only effect of these macros has been to make

>> unistd.h declare getlogin_r.  _POSIX_C_SOURCE >= 199506L also causes

>> this function to be declared.  However, people who don't carefully

>> read all the headers might be confused into thinking they need to

>> define _REENTRANT for any threaded code (as was indeed the case a long

>> time ago).

> 

> You should get rid of the internal #define and #undef of _REENTRANT, too.


Do you mean the ones in libc-symbols.h and check-installed-headers.sh?
Those are the only ones I see grepping.

This updated patch takes care of that and also the discussion in
creature.texi.  (creature.texi needs a thorough overhaul, but let's not
worry about that now.)  Also, it's been tested on x86-64-linux.

zw

Comments

Florian Weimer Dec. 7, 2016, 9:06 a.m. UTC | #1
On 12/06/2016 05:43 PM, Zack Weinberg wrote:
> On 12/06/2016 10:36 AM, Florian Weimer wrote:

>> On 12/06/2016 04:22 PM, Zack Weinberg wrote:

>>> For many years, the only effect of these macros has been to make

>>> unistd.h declare getlogin_r.  _POSIX_C_SOURCE >= 199506L also causes

>>> this function to be declared.  However, people who don't carefully

>>> read all the headers might be confused into thinking they need to

>>> define _REENTRANT for any threaded code (as was indeed the case a long

>>> time ago).

>>

>> You should get rid of the internal #define and #undef of _REENTRANT, too.

>

> Do you mean the ones in libc-symbols.h and check-installed-headers.sh?

> Those are the only ones I see grepping.


Right, those are the ones I noticed.

I agree that we probably have to remove the warning.

Thanks,
Florian
Zack Weinberg Dec. 7, 2016, 1:52 p.m. UTC | #2
On Wed, Dec 7, 2016 at 4:06 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 12/06/2016 05:43 PM, Zack Weinberg wrote:

>>

>> Do you mean the ones in libc-symbols.h and check-installed-headers.sh?

>> Those are the only ones I see grepping.

>

> Right, those are the ones I noticed.

>

> I agree that we probably have to remove the warning.


For clarity, both iterations of the patch warn *only* for cases where
_REENTRANT actually does something, which is only when, without it, we
would have activated a conformance level lower than
_POSIX_C_SOURCE=199506L.  This should be very rare.  In particular,
-D_REENTRANT by itself won't warn.  That was just me thinking out
loud.

I can see a case for not warning at all, though, what with -pthread
doing -D_REENTRANT.  -std=c89 -pthread without additional options is
probably wrong -- the code probably *wants* POSIX.1-2001 or higher
even if it doesn't know to ask for it -- but I can see it happening.
What do you think?

zw
Carlos O'Donell Dec. 8, 2016, 7:51 p.m. UTC | #3
On 12/06/2016 11:43 AM, Zack Weinberg wrote:
> On 12/06/2016 10:36 AM, Florian Weimer wrote:

>> On 12/06/2016 04:22 PM, Zack Weinberg wrote:

>>> For many years, the only effect of these macros has been to make

>>> unistd.h declare getlogin_r.  _POSIX_C_SOURCE >= 199506L also causes

>>> this function to be declared.  However, people who don't carefully

>>> read all the headers might be confused into thinking they need to

>>> define _REENTRANT for any threaded code (as was indeed the case a long

>>> time ago).

>>

>> You should get rid of the internal #define and #undef of _REENTRANT, too.

> 

> Do you mean the ones in libc-symbols.h and check-installed-headers.sh?

> Those are the only ones I see grepping.

> 

> This updated patch takes care of that and also the discussion in

> creature.texi.  (creature.texi needs a thorough overhaul, but let's not

> worry about that now.)  Also, it's been tested on x86-64-linux.


From a high level I support _anything_ that gets rid of _REENTRANT which is
by itself a conceptually flawed name [1].

The second version which fixes libc-symbols.h and check-installed-headers.sh
looks better, but I agree with Jopseh and Florian that it should not warn
for the variety of reasons already listed (compiler defaults being the
most important).

It looks like you have consensus on the change done by v2 patch but with
the warning disabled?

-- 
Cheers,
Carlos.

[1] https://sourceware.org/ml/libc-alpha/2014-12/msg00954.html
Zack Weinberg Dec. 8, 2016, 8:41 p.m. UTC | #4
On Thu, Dec 8, 2016 at 2:51 PM, Carlos O'Donell <carlos@redhat.com> wrote
>> This updated patch takes care of that and also the discussion in

>> creature.texi.  (creature.texi needs a thorough overhaul, but let's not

>> worry about that now.)  Also, it's been tested on x86-64-linux.

>

> From a high level I support _anything_ that gets rid of _REENTRANT which is

> by itself a conceptually flawed name [1].

>

> The second version which fixes libc-symbols.h and check-installed-headers.sh

> looks better, but I agree with Jopseh and Florian that it should not warn

> for the variety of reasons already listed (compiler defaults being the

> most important).

>

> It looks like you have consensus on the change done by v2 patch but with

> the warning disabled?


OK, I'm going to go ahead and land the revision I posted this morning
<https://sourceware.org/ml/libc-alpha/2016-12/msg00272.html> which has
the warning removed.

zw
diff mbox

Patch

From 8e04a5fe9858410311c71da92cfa119e2710213a Mon Sep 17 00:00:00 2001
From: Zack Weinberg <zackw@panix.com>
Date: Tue, 6 Dec 2016 10:14:42 -0500
Subject: [PATCH] Deprecate _REENTRANT and _THREAD_SAFE.

For many years, the only effect of these macros has been to make
unistd.h declare getlogin_r.  _POSIX_C_SOURCE >= 199506L also causes
this function to be declared.  However, people who don't carefully
read all the headers might be confused into thinking they need to
define _REENTRANT for any threaded code (as was indeed the case a long
time ago).

Therefore, I propose to make _REENTRANT and _THREAD_SAFE into aliases
for _POSIX_C_SOURCE=199506L, and issue a deprecation warning when a
higher conformance level was not selected some other way.
For instance, -std=c89 -D_REENTRANT will warn, but
-std=c99 -D_POSIX_C_SOURCE=200809L -D_REENTRANT won't, and
-D_REENTRANT all by itself also won't, because _DEFAULT_SOURCE implies
_POSIX_C_SOURCE > 199506.  I could be persuaded to make that final
case also warn.

	* NEWS: The feature selection macros _REENTRANT and _THREAD_SAFE
	are deprecated.
	* manual/creature.texi: Document them as obsolete.
	* include/features.h: Treat them as synonyms for
	_POSIX_C_SOURCE=199506L, if a higher POSIX conformance level
	has not been selected by other macros.

	* posix/unistd.h, posix/bits/unistd.h: Don't check __USE_REENTRANT.
	* include/libc-symbols.h: Don't define _REENTRANT.
	* scripts/check-installed-headers.sh: Don't undefine _REENTRANT.
---
 NEWS                               | 10 ++++++++++
 include/features.h                 | 39 ++++++++++++++++++++++++--------------
 include/libc-symbols.h             |  2 --
 manual/creature.texi               | 12 ++++--------
 posix/bits/unistd.h                |  2 +-
 posix/unistd.h                     |  2 +-
 scripts/check-installed-headers.sh |  1 -
 7 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/NEWS b/NEWS
index c2e973d..4f72149 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,16 @@  Version 2.25
   from that TS.  Note that most features from that TS are not supported by
   the GNU C Library.
 
+* The feature test macros _REENTRANT and _THREAD_SAFE are deprecated, and
+  will henceforth be treated as synonyms for _POSIX_C_SOURCE=199506L.
+  For many years, their only effect has been to cause <unistd.h> to declare
+  getlogin_r; _POSIX_C_SOURCE=199506L has the same effect.  Since the
+  GNU C Library defaults to a much newer revision of POSIX, this will
+  only affect programs that specifically request an old conformance mode.
+  (For instance, a program compiled with -std=c89 -D_REENTRANT will see a
+  change in the visible declarations, but a program compiled with
+  -std=c99 -D_POSIX_C_SOURCE=200809L -D_REENTRANT will not.)
+
 * The inclusion of <sys/sysmacros.h> by <sys/types.h> is deprecated.  This
   means that in a future release, the macros “major”, “minor”, and “makedev”
   will only be available from <sys/sysmacros.h>.
diff --git a/include/features.h b/include/features.h
index 650d4c5..5de950c 100644
--- a/include/features.h
+++ b/include/features.h
@@ -24,10 +24,13 @@ 
    __STRICT_ANSI__	ISO Standard C.
    _ISOC99_SOURCE	Extensions to ISO C89 from ISO C99.
    _ISOC11_SOURCE	Extensions to ISO C99 from ISO C11.
-   __STDC_WANT_LIB_EXT2__ Extensions to ISO C99 from TR 27431-2:2010.
-   __STDC_WANT_IEC_60559_BFP_EXT__ Extensions to ISO C11 from TS 18661-1:2014.
-   __STDC_WANT_IEC_60559_FUNCS_EXT__ Extensions to ISO C11 from
-			TS 18661-4:2015.
+   __STDC_WANT_LIB_EXT2__
+			Extensions to ISO C99 from TR 27431-2:2010.
+   __STDC_WANT_IEC_60559_BFP_EXT__
+			Extensions to ISO C11 from TS 18661-1:2014.
+   __STDC_WANT_IEC_60559_FUNCS_EXT__
+			Extensions to ISO C11 from TS 18661-4:2015.
+
    _POSIX_SOURCE	IEEE Std 1003.1.
    _POSIX_C_SOURCE	If ==1, like _POSIX_SOURCE; if >=2 add IEEE Std 1003.2;
 			if >=199309L, add IEEE Std 1003.1b-1993;
@@ -45,10 +48,12 @@ 
    _GNU_SOURCE		All of the above, plus GNU extensions.
    _DEFAULT_SOURCE	The default set of features (taking precedence over
 			__STRICT_ANSI__).
-   _REENTRANT		Select additionally reentrant object.
-   _THREAD_SAFE		Same as _REENTRANT, often used by other systems.
-   _FORTIFY_SOURCE	If set to numeric value > 0 additional security
-			measures are defined, according to level.
+
+   _FORTIFY_SOURCE	Add security hardening to many library functions.
+			Set to 1 or 2; 2 performs stricter checks than 1.
+
+   _REENTRANT, _THREAD_SAFE
+			Obsolete; equivalent to _POSIX_C_SOURCE=199506L.
 
    The `-ansi' switch to the GNU C compiler, and standards conformance
    options such as `-std=c99', define __STRICT_ANSI__.  If none of
@@ -86,7 +91,6 @@ 
    __USE_MISC		Define things from 4.3BSD or System V Unix.
    __USE_ATFILE		Define *at interfaces and AT_* constants for them.
    __USE_GNU		Define GNU extensions.
-   __USE_REENTRANT	Define reentrant/thread-safe *_r functions.
    __USE_FORTIFY_LEVEL	Additional security measures used, according to level.
 
    The macros `__GNU_LIBRARY__', `__GLIBC__', and `__GLIBC_MINOR__' are
@@ -130,7 +134,6 @@ 
 #undef	__USE_MISC
 #undef	__USE_ATFILE
 #undef	__USE_GNU
-#undef	__USE_REENTRANT
 #undef	__USE_FORTIFY_LEVEL
 #undef	__KERNEL_STRICT_NAMES
 
@@ -253,6 +256,7 @@ 
 # undef  _POSIX_C_SOURCE
 # define _POSIX_C_SOURCE	200809L
 #endif
+
 #if ((!defined __STRICT_ANSI__					\
       || (defined _XOPEN_SOURCE && (_XOPEN_SOURCE - 0) >= 500))	\
      && !defined _POSIX_SOURCE && !defined _POSIX_C_SOURCE)
@@ -269,6 +273,17 @@ 
 # define __USE_POSIX_IMPLICITLY	1
 #endif
 
+/* _REENTRANT and _THREAD_SAFE are obsolete aliases for _POSIX_C_SOURCE=199506L.
+   Only issue a warning if _POSIX_C_SOURCE was not already defined to
+   an equivalent or higher level some other way.  */
+#if ((!defined _POSIX_C_SOURCE || (_POSIX_C_SOURCE - 0) < 199506L) \
+     && (defined _REENTRANT || defined _THREAD_SAFE))
+# warning "_REENTRANT and _THREAD_SAFE are deprecated, use _POSIX_C_SOURCE"
+# define _POSIX_SOURCE   1
+# undef  _POSIX_C_SOURCE
+# define _POSIX_C_SOURCE 199506L
+#endif
+
 #if (defined _POSIX_SOURCE					\
      || (defined _POSIX_C_SOURCE && _POSIX_C_SOURCE >= 1)	\
      || defined _XOPEN_SOURCE)
@@ -351,10 +366,6 @@ 
 # define __USE_GNU	1
 #endif
 
-#if defined _REENTRANT || defined _THREAD_SAFE
-# define __USE_REENTRANT	1
-#endif
-
 #if defined _FORTIFY_SOURCE && _FORTIFY_SOURCE > 0
 # if !defined __OPTIMIZE__ || __OPTIMIZE__ <= 0
 #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index 1c91f2e..4238d79 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -52,8 +52,6 @@ 
 
 /* Enable declarations of GNU extensions, since we are compiling them.  */
 #define _GNU_SOURCE	1
-/* And we also need the data for the reentrant functions.  */
-#define _REENTRANT	1
 
 #include <config.h>
 
diff --git a/manual/creature.texi b/manual/creature.texi
index 257f871..5048886 100644
--- a/manual/creature.texi
+++ b/manual/creature.texi
@@ -219,15 +219,11 @@  cause them to be disabled.
 @comment GNU
 @defvr Macro _REENTRANT
 @defvrx Macro _THREAD_SAFE
-If you define one of these macros, reentrant versions of several functions get
-declared.  Some of the functions are specified in POSIX.1c but many others
-are only available on a few other systems or are unique to @theglibc{}.
-The problem is the delay in the standardization of the thread safe C library
-interface.
+These macros are obsolete.  They have the same effect as defining
+@code{_POSIX_C_SOURCE} with the value @code{199506L}.
 
-Unlike on some other systems, no special version of the C library must be
-used for linking.  There is only one version but while compiling this
-it must have been specified to compile as thread safe.
+Some very old C libraries required one of these macros to be defined
+for basic functionality (e.g.@: @code{getchar}) to be thread-safe.
 @end defvr
 
 We recommend you use @code{_GNU_SOURCE} in new programs.  If you don't
diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
index 0105f04..4a5f42e 100644
--- a/posix/bits/unistd.h
+++ b/posix/bits/unistd.h
@@ -302,7 +302,7 @@  __NTH (ttyname_r (int __fd, char *__buf, size_t __buflen))
 }
 
 
-#if defined __USE_REENTRANT || defined __USE_POSIX199506
+#ifdef __USE_POSIX199506
 extern int __getlogin_r_chk (char *__buf, size_t __buflen, size_t __nreal)
      __nonnull ((1));
 extern int __REDIRECT (__getlogin_r_alias, (char *__buf, size_t __buflen),
diff --git a/posix/unistd.h b/posix/unistd.h
index 625ba77..392dc0e 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -849,7 +849,7 @@  extern int tcsetpgrp (int __fd, __pid_t __pgrp_id) __THROW;
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
 extern char *getlogin (void);
-#if defined __USE_REENTRANT || defined __USE_POSIX199506
+#ifdef __USE_POSIX199506
 /* Return at most NAME_LEN characters of the login name of the user in NAME.
    If it cannot be determined or some other error occurred, return the error
    code.  Otherwise return 0.
diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh
index a245fe6..89e1514 100644
--- a/scripts/check-installed-headers.sh
+++ b/scripts/check-installed-headers.sh
@@ -143,7 +143,6 @@  EOF
    inappropriate for this test.  */
 #undef _LIBC
 #undef _GNU_SOURCE
-#undef _REENTRANT
 /* The library mode is selected here rather than on the command line to
    ensure that this selection wins. */
 $expanded_lib_mode
-- 
2.10.2