Re: [PATCH] features.h: do not consider _GNU_SOURCE for gets

Arsen Arsenović <[email protected]> writes:

> Currently, -std=c99 -D_GNU_SOURCE and similar lack the 'gets'
> declaration.
>
> This causes, say, programs using the:
>
>   extern __typeof__ (gets) function __attribute__ ((__warning__ (...)))
>
> ... trick to add warnings to 'gets' use to break.  (_GL_WARN_ON_USE in
> gnulib).
>
> The decision not to declare 'gets' w/ _GNU_SOURCE is clearly intended
> (see c3a87236702cb73be1dada3438bbd3c3934e83f8), but I don't see the
> benefit in it (given that we already emit a diagnostic), and, as noted,
> it does cause certain older versions of Gnulib to fail to compile
> (which, in turn, causes multiple packages to fail to compile while
> building), and no rationale was provided in the commit message.

Interestingly, nobody questioned it in the followup at
https://inbox.sourceware.org/libc-alpha/[email protected]/
and indeed nobody really argued in
https://inbox.sourceware.org/libc-alpha/CAOPLpQeb2HAkBVmoyOVrTsHtT6tG2ECEMBrK74zZV0Xw2243yg@mail.gmail.com/
either.

I feel like I've seen some discussion of this over the years but I can't
find it now, so maybe imagined it.

>
> It was not unreasonable for programs to assume that _GNU_SOURCE does not
> remove declarations, I think.

I agree that it's rather counterintuitive. I think adding (even
aggressive) warnings is arguably in scope for _GNU_SOURCE but outright
hiding a fnuction isn't.

It encourages people to redefine it incorrectly anyway and I'd argue is
harmful in getting people to use feature test macros correctly.

>
> So, let's keep it in -std=gnu99/c99 -D_GNU_SOURCE instead.
>
> One test needed to ignore the 'gets' deprecation now, since it became
> exposed to the deprecated declaration.
> ---
> Tested on x86_64-linux-gnu.
>
> Four failures that seem to be definitely unrelated (the GDB ones are a
> GDB crash, the malloc one is a timeout, and tst-pthread-exited seems to
> get EPERMs for some reason):
>
>   ~/gnu/glibc/glibc/_b$ grep ^FAIL tests.sum
>   FAIL: malloc/tst-malloc-tcache-leak-malloc-largetcache
>   FAIL: nptl/tst-pthread-exited
>   FAIL: nptl/tst-pthread-gdb-attach
>   FAIL: nptl/tst-pthread-gdb-attach-static
>
> I'd debug this normally, but it's getting late..
>
>  include/features.h | 4 +++-
>  libio/Makefile     | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/features.h b/include/features.h
> index b3277f342cd6..a3f483fa7869 100644
> --- a/include/features.h
> +++ b/include/features.h
> @@ -475,7 +475,9 @@
>     safely.  It has been removed from ISO C11 and ISO C++14.  Note: for
>     compatibility with various implementations of <cstdio>, this test
>     must consider only the value of __cplusplus when compiling C++.  */
> -#if defined __cplusplus ? __cplusplus >= 201402L : defined __USE_ISOC11
> +#if (defined __cplusplus			\
> +     ? __cplusplus >= 201402L			\
> +     : (defined __STDC_VERSION__ && __STDC_VERSION__ >= 201112L))
>  # define __GLIBC_USE_DEPRECATED_GETS 0
>  #else
>  # define __GLIBC_USE_DEPRECATED_GETS 1
> diff --git a/libio/Makefile b/libio/Makefile
> index 616107ee105b..836a9dbb70d4 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -258,7 +258,7 @@ CFLAGS-oldtmpfile.c += -fexceptions
>  
>  # Prevent fortification as these are built with -O0
>  CFLAGS-tst-bz24051.c += $(no-fortify-source)
> -CFLAGS-tst-bz24153.c += $(no-fortify-source)
> +CFLAGS-tst-bz24153.c += $(no-fortify-source) -Wno-deprecated-declarations
>  
>  CFLAGS-tst_putwc.c += -DOBJPFX=\"$(objpfx)\"

-----BEGIN PGP SIGNATURE-----

iQEBBAEWCgCpFiEEJaa7iN2bdkxrVUHCc4QJ9SDfkZAFAmqA/c8bFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMiwyXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25z
Lm9wZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQyNUE2QkI4OEREOUI3NjRDNkI1NTQx
QzI3Mzg0MDlGNTIwREY5MTkwDxxzYW1AZ2VudG9vLm9yZwAKCRBzhAn1IN+RkLun
AP4prLHp85Rppa6Yhh68wQPWTUTHH3h5hWu0s7BZHV9vsAD/S3uxubj9vtpgVMk0
/nJE9C+14sOZpTLDFKdDw2+oTA4=
=l35i
-----END PGP SIGNATURE-----