diff mbox series

system: Disable gcc warning on sigsetmask

Message ID 20181119104711.vsdh4tj3joeje65k@gondor.apana.org.au (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series system: Disable gcc warning on sigsetmask | expand

Commit Message

Herbert Xu Nov. 19, 2018, 10:47 a.m. UTC
Jilles Tjoelker <jilles@stack.nl> wrote:
>
> The git history starts (in 2005) after HAVE_SIGSETMASK was added, but I
> expect it was done this way to save a few bytes in the executable. With
> ProPolice, the effect may be a bit more since a local variable of type
> sigset_t often contains an array and may cause functions to be compiled
> with stack protection when they otherwise wouldn't (note that
> sigclearmask() is inline).
> 
> Both FreeBSD and NetBSD simply changed the sigsetmask() call to a
> sigprocmask() call very early on (1995-1996).
> 
> Personally, I think clean code that compiles without warnings is more
> important than making the executable as small as possible, but the
> maintainer may not agree.

I agree with getting rid of the warning, and this seems to work
for me.  Please let me know if you still a get warning on some
other platform.  Thanks!

---8<---
As sigsetmask is set as deprecated in glibc this patch adds the
pragmas to disable the warning in gcc around our one and only use
of sigsetmask.

Reported-by: Antonio Ospite <ao2@ao2.it>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

Antonio Ospite Nov. 19, 2018, 11:51 a.m. UTC | #1
On Mon, 19 Nov 2018 18:47:11 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Jilles Tjoelker <jilles@stack.nl> wrote:
> >
> > The git history starts (in 2005) after HAVE_SIGSETMASK was added, but I
> > expect it was done this way to save a few bytes in the executable. With
> > ProPolice, the effect may be a bit more since a local variable of type
> > sigset_t often contains an array and may cause functions to be compiled
> > with stack protection when they otherwise wouldn't (note that
> > sigclearmask() is inline).
> > 
> > Both FreeBSD and NetBSD simply changed the sigsetmask() call to a
> > sigprocmask() call very early on (1995-1996).
> > 
> > Personally, I think clean code that compiles without warnings is more
> > important than making the executable as small as possible, but the
> > maintainer may not agree.
> 
> I agree with getting rid of the warning, and this seems to work
> for me.  Please let me know if you still a get warning on some
> other platform.  Thanks!
> 
> ---8<---
> As sigsetmask is set as deprecated in glibc this patch adds the
> pragmas to disable the warning in gcc around our one and only use
> of sigsetmask.
>

Hi Herbert,

this would work for me, thank you.

Just to double check: you prefer to keep sigsetmask() around as an
optimization and NOT for compatibility reasons, right?

If so, it might be useful to mention in the commit message why you
opted for masking the warning instead of removing the usage of the
deprecated function.

Without context future readers might argue that it would have been
possible to remove sigsetmask() without sacrificing compatibility, as
sigprocmask() and friends are already used in src/trap.c too anyway.

Ciao,
   Antonio

> Reported-by: Antonio Ospite <ao2@ao2.it>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/src/system.h b/src/system.h
> index a8d09b3..f3aa930 100644
> --- a/src/system.h
> +++ b/src/system.h
> @@ -37,7 +37,14 @@
>  static inline void sigclearmask(void)
>  {
>  #ifdef HAVE_SIGSETMASK
> +#if defined(__GNUC__) && (__GNUC__ * 1000 + __GNUC_MINOR__) >= 4006
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +#endif
>  	sigsetmask(0);
> +#if defined(__GNUC__) && (__GNUC__ * 1000 + __GNUC_MINOR__) >= 4006
> +#pragma GCC diagnostic pop
> +#endif
>  #else
>  	sigset_t set;
>  	sigemptyset(&set);
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Antonio Ospite Nov. 20, 2018, 10:48 p.m. UTC | #2
On Mon, 19 Nov 2018 18:47:11 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

[...]
> I agree with getting rid of the warning, and this seems to work
> for me.  Please let me know if you still a get warning on some
> other platform.  Thanks!
>

Just for the record, compiling with clang (CC=clang ./configure && make)
still gives a warning:

-----------------------------------------------------------------------
  CC       system.o
In file included from system.c:62:
./system.h:44:2: warning: 'sigsetmask' is deprecated [-Wdeprecated-declarations]
        sigsetmask(0);
        ^
/usr/include/signal.h:173:44: note: 'sigsetmask' has been explicitly marked deprecated here
extern int sigsetmask (int __mask) __THROW __attribute_deprecated__;
                                           ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:246:51: note: expanded from macro '__attribute_deprecated__'
# define __attribute_deprecated__ __attribute__ ((__deprecated__))
                                                  ^
1 warning generated.
-----------------------------------------------------------------------

This is expected as the patch below only addresses gcc.

If there is a valid reason to keep sigsetmask() around I think we can
live with some warnings in unusual setups, but if the gain is negligible
we might as well just get rid of it.

Thank you,
   Antonio

> ---8<---
> As sigsetmask is set as deprecated in glibc this patch adds the
> pragmas to disable the warning in gcc around our one and only use
> of sigsetmask.
> 
> Reported-by: Antonio Ospite <ao2@ao2.it>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> diff --git a/src/system.h b/src/system.h
> index a8d09b3..f3aa930 100644
> --- a/src/system.h
> +++ b/src/system.h
> @@ -37,7 +37,14 @@
>  static inline void sigclearmask(void)
>  {
>  #ifdef HAVE_SIGSETMASK
> +#if defined(__GNUC__) && (__GNUC__ * 1000 + __GNUC_MINOR__) >= 4006
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +#endif
>  	sigsetmask(0);
> +#if defined(__GNUC__) && (__GNUC__ * 1000 + __GNUC_MINOR__) >= 4006
> +#pragma GCC diagnostic pop
> +#endif
>  #else
>  	sigset_t set;
>  	sigemptyset(&set);
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff mbox series

Patch

diff --git a/src/system.h b/src/system.h
index a8d09b3..f3aa930 100644
--- a/src/system.h
+++ b/src/system.h
@@ -37,7 +37,14 @@ 
 static inline void sigclearmask(void)
 {
 #ifdef HAVE_SIGSETMASK
+#if defined(__GNUC__) && (__GNUC__ * 1000 + __GNUC_MINOR__) >= 4006
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
 	sigsetmask(0);
+#if defined(__GNUC__) && (__GNUC__ * 1000 + __GNUC_MINOR__) >= 4006
+#pragma GCC diagnostic pop
+#endif
 #else
 	sigset_t set;
 	sigemptyset(&set);