diff mbox series

V2 [PATCH] Deprecate SIGSTKSZ/MINSIGSTKSZ with _SC_SIGSTKSZ_SOURCE

Message ID CAMe9rOo3FdwJV8fa=KcHa6VSqNexzgiiF2NS1uq=ay+bzMHpOw@mail.gmail.com
State Superseded
Headers
Series V2 [PATCH] Deprecate SIGSTKSZ/MINSIGSTKSZ with _SC_SIGSTKSZ_SOURCE |

Commit Message

H.J. Lu Oct. 15, 2020, 9:22 p.m. UTC 
  On Thu, Oct 15, 2020 at 12:59 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 15 Oct 2020, H.J. Lu via Libc-alpha wrote:
>
> > +__attribute_deprecated_msg__ ("Use sysconf (_SC_SIGSTKSZ) instead")
> > +__extern_always_inline long
> > +SIGSTKSZ_is_deprecated (void)
> > +{
> > +  return sysconf (_SC_SIGSTKSZ);
> > +}
> > +
> > +__attribute_deprecated_msg__ ("Use sysconf (_SC_MINSIGSTKSZ) instead")
> > +__extern_always_inline long
> > +MINSIGSTKSZ_is_deprecated (void)
> > +{
> > +  return sysconf (_SC_SIGSTKSZ);
> > +}
>
> Those function names should start with '__' rather than claiming the names
> SIGSTKSZ_is_deprecated and MINSIGSTKSZ_is_deprecated from the user's
> namespace.
>

Like this?

Comments

Joseph Myers Oct. 16, 2020, 12:57 a.m. UTC | #1 
On Thu, 15 Oct 2020, H.J. Lu via Libc-alpha wrote:

> > Those function names should start with '__' rather than claiming the names
> > SIGSTKSZ_is_deprecated and MINSIGSTKSZ_is_deprecated from the user's
> > namespace.
> >
> 
> Like this?

Yes (this is not a review of this patch, but, if we do want such a 
deprecation, the function names used in this patch are now appropriate).
Carlos O'Donell July 9, 2021, 6:53 p.m. UTC | #2 
On 10/15/20 5:22 PM, H.J. Lu via Libc-alpha wrote:
> From a9b02d82571c0b0e0a5d4b46f0683a5341046bdf Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 15 Oct 2020 05:21:28 -0700
> Subject: [PATCH] Deprecate SIGSTKSZ/MINSIGSTKSZ with _SC_SIGSTKSZ_SOURCE
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> When _SC_SIGSTKSZ_SOURCE is defined, deprecate SIGSTKSZ and MINSIGSTKSZ:

This is approved for glibc 2.35 when the tree reopens.

It is entirely my fault this won't make it into 2.34 because I've been
prioritizing other ABI changes. Sorry for that.

My feeling is that we should kick off the deprecation, allow 6 months for
downstreams to catch FTBS, and send patches upstream for inclusion in
subsequent distros.

Please commit this when we reopen the branch for 2.35.

I also think that for 2.35 we could consider PTHREAD_STACK_MIN deprecated
in exactly the same way and at the same time? I have reviewed the PTHREAD_STACK_MIN
non-constant changes just now.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
 
> tst-minsigstksz-5.c: In function ‘do_test’:
> tst-minsigstksz-5.c:46:3: warning: ‘__MINSIGSTKSZ’ is deprecated: Use sysconf (_SC_MINSIGSTKSZ) instead of MINSIGSTKSZ [-Wdeprecated-declarations]
>    46 |   void *stack_bottom = stack_buffer + (stack_buffer_size + MINSIGSTKSZ) / 2;
>       |   ^~~~
> In file included from ../signal/signal.h:315,
>                  from ../include/signal.h:2,
>                  from tst-minsigstksz-5.c:19:
> ../sysdeps/unix/sysv/linux/bits/sigstksz.h:35:1: note: declared here
>    35 | __MINSIGSTKSZ (void)
>       | ^~~~~~~~~~~~~
> ---
>  sysdeps/unix/sysv/linux/Makefile        |  6 ++++--
>  sysdeps/unix/sysv/linux/bits/sigstksz.h | 18 ++++++++++++++++--
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index b51a02a6e6..5576c729ae 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -189,9 +189,11 @@ sysdep_headers += sys/timex.h bits/timex.h
>  sysdep_routines += ntp_gettime ntp_gettimex
>  endif
>  
> +CFLAGS-SIGSTKSZ += -D_SC_SIGSTKSZ_SOURCE -Wno-error=deprecated-declarations
> +
>  ifeq ($(subdir),signal)
>  # Compile tst-minsigstksz-5.c with _SC_SIGSTKSZ_SOURCE.
> -CFLAGS-tst-minsigstksz-5.c += -D_SC_SIGSTKSZ_SOURCE
> +CFLAGS-tst-minsigstksz-5.c += $(CFLAGS-SIGSTKSZ)
>  
>  tests-special += $(objpfx)tst-signal-numbers.out
>  # Depending on signal.o* is a hack.  What we actually want is a dependency
> @@ -233,7 +235,7 @@ endif
>  
>  ifeq ($(subdir),support)
>  # Compile xsigstack.c with _SC_SIGSTKSZ_SOURCE.
> -CFLAGS-xsigstack.c += -D_SC_SIGSTKSZ_SOURCE
> +CFLAGS-xsigstack.c += $(CFLAGS-SIGSTKSZ)
>  endif
>  
>  ifeq ($(subdir),termios)
> diff --git a/sysdeps/unix/sysv/linux/bits/sigstksz.h b/sysdeps/unix/sysv/linux/bits/sigstksz.h
> index cd5b3cc895..b90a614de6 100644
> --- a/sysdeps/unix/sysv/linux/bits/sigstksz.h
> +++ b/sysdeps/unix/sysv/linux/bits/sigstksz.h
> @@ -23,11 +23,25 @@
>  #if __USE_SC_SIGSTKSZ
>  # include <unistd.h>
>  
> +__attribute_deprecated_msg__ ("Use sysconf (_SC_SIGSTKSZ) instead of SIGSTKSZ")
> +__extern_always_inline long
> +__SIGSTKSZ (void)
> +{
> +  return sysconf (_SC_SIGSTKSZ);
> +}
> +
> +__attribute_deprecated_msg__ ("Use sysconf (_SC_MINSIGSTKSZ) instead of MINSIGSTKSZ")
> +__extern_always_inline long
> +__MINSIGSTKSZ (void)
> +{
> +  return sysconf (_SC_SIGSTKSZ);
> +}
> +
>  /* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
>  # undef SIGSTKSZ
> -# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
> +# define SIGSTKSZ __SIGSTKSZ ()
>  
>  /* Minimum stack size for a signal handler: SIGSTKSZ.  */
>  # undef MINSIGSTKSZ
> -# define MINSIGSTKSZ SIGSTKSZ
> +# define MINSIGSTKSZ __MINSIGSTKSZ ()
>  #endif
> -- 
> 2.26.2
>
H.J. Lu July 9, 2021, 7:34 p.m. UTC | #3 
On Fri, Jul 9, 2021 at 11:53 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 10/15/20 5:22 PM, H.J. Lu via Libc-alpha wrote:
> > From a9b02d82571c0b0e0a5d4b46f0683a5341046bdf Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 15 Oct 2020 05:21:28 -0700
> > Subject: [PATCH] Deprecate SIGSTKSZ/MINSIGSTKSZ with _SC_SIGSTKSZ_SOURCE
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > When _SC_SIGSTKSZ_SOURCE is defined, deprecate SIGSTKSZ and MINSIGSTKSZ:
>
> This is approved for glibc 2.35 when the tree reopens.
>
> It is entirely my fault this won't make it into 2.34 because I've been
> prioritizing other ABI changes. Sorry for that.
>
> My feeling is that we should kick off the deprecation, allow 6 months for
> downstreams to catch FTBS, and send patches upstream for inclusion in
> subsequent distros.
>
> Please commit this when we reopen the branch for 2.35.

Will do after 2.34 is branched.

> I also think that for 2.35 we could consider PTHREAD_STACK_MIN deprecated
> in exactly the same way and at the same time? I have reviewed the PTHREAD_STACK_MIN
> non-constant changes just now.

I will submit a patch for PTHREAD_STACK_MIN after this one.

Thanks.

> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > tst-minsigstksz-5.c: In function ‘do_test’:
> > tst-minsigstksz-5.c:46:3: warning: ‘__MINSIGSTKSZ’ is deprecated: Use sysconf (_SC_MINSIGSTKSZ) instead of MINSIGSTKSZ [-Wdeprecated-declarations]
> >    46 |   void *stack_bottom = stack_buffer + (stack_buffer_size + MINSIGSTKSZ) / 2;
> >       |   ^~~~
> > In file included from ../signal/signal.h:315,
> >                  from ../include/signal.h:2,
> >                  from tst-minsigstksz-5.c:19:
> > ../sysdeps/unix/sysv/linux/bits/sigstksz.h:35:1: note: declared here
> >    35 | __MINSIGSTKSZ (void)
> >       | ^~~~~~~~~~~~~
> > ---
> >  sysdeps/unix/sysv/linux/Makefile        |  6 ++++--
> >  sysdeps/unix/sysv/linux/bits/sigstksz.h | 18 ++++++++++++++++--
> >  2 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> > index b51a02a6e6..5576c729ae 100644
> > --- a/sysdeps/unix/sysv/linux/Makefile
> > +++ b/sysdeps/unix/sysv/linux/Makefile
> > @@ -189,9 +189,11 @@ sysdep_headers += sys/timex.h bits/timex.h
> >  sysdep_routines += ntp_gettime ntp_gettimex
> >  endif
> >
> > +CFLAGS-SIGSTKSZ += -D_SC_SIGSTKSZ_SOURCE -Wno-error=deprecated-declarations
> > +
> >  ifeq ($(subdir),signal)
> >  # Compile tst-minsigstksz-5.c with _SC_SIGSTKSZ_SOURCE.
> > -CFLAGS-tst-minsigstksz-5.c += -D_SC_SIGSTKSZ_SOURCE
> > +CFLAGS-tst-minsigstksz-5.c += $(CFLAGS-SIGSTKSZ)
> >
> >  tests-special += $(objpfx)tst-signal-numbers.out
> >  # Depending on signal.o* is a hack.  What we actually want is a dependency
> > @@ -233,7 +235,7 @@ endif
> >
> >  ifeq ($(subdir),support)
> >  # Compile xsigstack.c with _SC_SIGSTKSZ_SOURCE.
> > -CFLAGS-xsigstack.c += -D_SC_SIGSTKSZ_SOURCE
> > +CFLAGS-xsigstack.c += $(CFLAGS-SIGSTKSZ)
> >  endif
> >
> >  ifeq ($(subdir),termios)
> > diff --git a/sysdeps/unix/sysv/linux/bits/sigstksz.h b/sysdeps/unix/sysv/linux/bits/sigstksz.h
> > index cd5b3cc895..b90a614de6 100644
> > --- a/sysdeps/unix/sysv/linux/bits/sigstksz.h
> > +++ b/sysdeps/unix/sysv/linux/bits/sigstksz.h
> > @@ -23,11 +23,25 @@
> >  #if __USE_SC_SIGSTKSZ
> >  # include <unistd.h>
> >
> > +__attribute_deprecated_msg__ ("Use sysconf (_SC_SIGSTKSZ) instead of SIGSTKSZ")
> > +__extern_always_inline long
> > +__SIGSTKSZ (void)
> > +{
> > +  return sysconf (_SC_SIGSTKSZ);
> > +}
> > +
> > +__attribute_deprecated_msg__ ("Use sysconf (_SC_MINSIGSTKSZ) instead of MINSIGSTKSZ")
> > +__extern_always_inline long
> > +__MINSIGSTKSZ (void)
> > +{
> > +  return sysconf (_SC_SIGSTKSZ);
> > +}
> > +
> >  /* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
> >  # undef SIGSTKSZ
> > -# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
> > +# define SIGSTKSZ __SIGSTKSZ ()
> >
> >  /* Minimum stack size for a signal handler: SIGSTKSZ.  */
> >  # undef MINSIGSTKSZ
> > -# define MINSIGSTKSZ SIGSTKSZ
> > +# define MINSIGSTKSZ __MINSIGSTKSZ ()
> >  #endif
> > --
> > 2.26.2
> >
>
>
> --
> Cheers,
> Carlos.
>
diff mbox series

Patch

From a9b02d82571c0b0e0a5d4b46f0683a5341046bdf Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 15 Oct 2020 05:21:28 -0700
Subject: [PATCH] Deprecate SIGSTKSZ/MINSIGSTKSZ with _SC_SIGSTKSZ_SOURCE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When _SC_SIGSTKSZ_SOURCE is defined, deprecate SIGSTKSZ and MINSIGSTKSZ:

tst-minsigstksz-5.c: In function ‘do_test’:
tst-minsigstksz-5.c:46:3: warning: ‘__MINSIGSTKSZ’ is deprecated: Use sysconf (_SC_MINSIGSTKSZ) instead of MINSIGSTKSZ [-Wdeprecated-declarations]
   46 |   void *stack_bottom = stack_buffer + (stack_buffer_size + MINSIGSTKSZ) / 2;
      |   ^~~~
In file included from ../signal/signal.h:315,
                 from ../include/signal.h:2,
                 from tst-minsigstksz-5.c:19:
../sysdeps/unix/sysv/linux/bits/sigstksz.h:35:1: note: declared here
   35 | __MINSIGSTKSZ (void)
      | ^~~~~~~~~~~~~
---
 sysdeps/unix/sysv/linux/Makefile        |  6 ++++--
 sysdeps/unix/sysv/linux/bits/sigstksz.h | 18 ++++++++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index b51a02a6e6..5576c729ae 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -189,9 +189,11 @@  sysdep_headers += sys/timex.h bits/timex.h
 sysdep_routines += ntp_gettime ntp_gettimex
 endif
 
+CFLAGS-SIGSTKSZ += -D_SC_SIGSTKSZ_SOURCE -Wno-error=deprecated-declarations
+
 ifeq ($(subdir),signal)
 # Compile tst-minsigstksz-5.c with _SC_SIGSTKSZ_SOURCE.
-CFLAGS-tst-minsigstksz-5.c += -D_SC_SIGSTKSZ_SOURCE
+CFLAGS-tst-minsigstksz-5.c += $(CFLAGS-SIGSTKSZ)
 
 tests-special += $(objpfx)tst-signal-numbers.out
 # Depending on signal.o* is a hack.  What we actually want is a dependency
@@ -233,7 +235,7 @@  endif
 
 ifeq ($(subdir),support)
 # Compile xsigstack.c with _SC_SIGSTKSZ_SOURCE.
-CFLAGS-xsigstack.c += -D_SC_SIGSTKSZ_SOURCE
+CFLAGS-xsigstack.c += $(CFLAGS-SIGSTKSZ)
 endif
 
 ifeq ($(subdir),termios)
diff --git a/sysdeps/unix/sysv/linux/bits/sigstksz.h b/sysdeps/unix/sysv/linux/bits/sigstksz.h
index cd5b3cc895..b90a614de6 100644
--- a/sysdeps/unix/sysv/linux/bits/sigstksz.h
+++ b/sysdeps/unix/sysv/linux/bits/sigstksz.h
@@ -23,11 +23,25 @@ 
 #if __USE_SC_SIGSTKSZ
 # include <unistd.h>
 
+__attribute_deprecated_msg__ ("Use sysconf (_SC_SIGSTKSZ) instead of SIGSTKSZ")
+__extern_always_inline long
+__SIGSTKSZ (void)
+{
+  return sysconf (_SC_SIGSTKSZ);
+}
+
+__attribute_deprecated_msg__ ("Use sysconf (_SC_MINSIGSTKSZ) instead of MINSIGSTKSZ")
+__extern_always_inline long
+__MINSIGSTKSZ (void)
+{
+  return sysconf (_SC_SIGSTKSZ);
+}
+
 /* Default stack size for a signal handler: sysconf (SC_SIGSTKSZ).  */
 # undef SIGSTKSZ
-# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
+# define SIGSTKSZ __SIGSTKSZ ()
 
 /* Minimum stack size for a signal handler: SIGSTKSZ.  */
 # undef MINSIGSTKSZ
-# define MINSIGSTKSZ SIGSTKSZ
+# define MINSIGSTKSZ __MINSIGSTKSZ ()
 #endif
-- 
2.26.2