]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
RFC: Use anonymous union for siginfo_t zack/anon-unions
authorZack Weinberg <zackw@panix.com>
Fri, 11 Aug 2017 13:43:59 +0000 (09:43 -0400)
committerZack Weinberg <zackw@panix.com>
Wed, 23 Aug 2017 11:42:53 +0000 (07:42 -0400)
C2011 officially includes an 'anonymous union' feature, and GCC has
supported it for many years.  It makes sub-fields of a union that's a
struct field appear to be fields of the parent struct.  If we use this
in the definition of siginfo_t, we don't need to define lots of
innocuous-looking identifiers like 'si_pid' as macros expanding to
chains of field accessors.  The catch, however, is that the compiler
used to compile *programs that use glibc* - not just glibc itself -
must accept the use of this feature (in system headers) even when
running in an older conformance mode.

This patch only touches siginfo_t, but if people like the idea, we
could also do it for several other types:

netinet/in.h (in6_addr)
sys/stat.h (stat)
utmp.h (utmp)
signal.h (sigaction, sigevent_t)
ucontext.h (ucontext_t)

and maybe also - these use names in the user namespace for the fields
that would be removed:

net/if.h (ifaddr)
ifaddrs.h (ifaddrs)
netinet/in6.h (ip6_hdr) (really should be bitfields instead)
netinet/icmp6.h (many)
net/if_ppp.h (ifpppstatsreq, ifpppcstatsreq)
net/if_shaper.h (shaperconf)
a.out.h (exec) (only some versions)
sys/quota.h (dqblk?) (the dq_* macros refer to a field that doesn't exist?!)

There are still more hits in sunrpc and nis, but since hopefully that
code is going to go away, I don't propose to mess with them.  And
there may be even more that aren't caught by grepping for
'#define IDENT IDENT.IDENT'.

As a side note (and this could be split for commit if felt
appropriate), the siginfo_t field aliases 'si_int' and 'si_ptr' are
not in POSIX.  There are a few uses of these within glibc itself, and
a handful more in third-party software (not glibc, not uclibc, and not
linux) so I have preserved them, but put them under __USE_MISC and
added a deprecation warning.

This passes the glibc testsuite on x86-64-linux, which probably
*doesn't* test the case where someone is compiling a program in
an older conformance mode that uses siginfo_t
(-std=c99 -D_XOPEN_SOURCE=600, perhaps).

What do you think?

zw

* sysdeps/unix/sysv/linux/bits/types/siginfo_t.h (siginfo_t):
Use C2011 anonymous union and anonymous struct-in-union features
to define this type.  Rename some public fields with their
official names.
(si_pid, si_uid, si_timerid, si_overrun, si_status, si_utime)
(si_stime, si_value, si_addr, si_addr_lsb, si_lower, si_upper)
(si_pkey, si_band, si_fd, si_call_addr, si_syscall, si_arch):
Do not define as macros.
(si_int, si_ptr): Define only when __USE_MISC, with deprecation
warnings.
* sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h
* sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h
* sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h
(__SI_SIGFAULT_ADDL): Define all fields with their public names
when __USE_GNU, or with impl-namespace names otherwise.
(si_imm, si_segvflags, si_isr, si_trapno): Do not define as macros.

* sysdeps/unix/sysv/linux/timer_routines.c (timer_helper_thread)
Use si_value.sival_ptr instead of si_ptr.
* sysdeps/unix/sysv/linux/tst-getpid1.c (do_test):
Use si_value.sival_int instead of si_int.

NEWS
sysdeps/unix/sysv/linux/bits/types/siginfo_t.h
sysdeps/unix/sysv/linux/ia64/bits/siginfo-arch.h
sysdeps/unix/sysv/linux/sparc/bits/siginfo-arch.h
sysdeps/unix/sysv/linux/tile/bits/siginfo-arch.h
sysdeps/unix/sysv/linux/timer_routines.c
sysdeps/unix/sysv/linux/tst-getpid1.c

diff --git a/NEWS b/NEWS
index 8fe0879bc44063d8d97fa5589b3a16d2d0606b68..482e355f4ffcc502950cf123d790cbf6c9a76ab7 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,9 @@ Deprecated and removed features, and other changes affecting compatibility:
 
 * On GNU/Linux, the obsolete Linux constant PTRACE_SEIZE_DEVEL is no longer
   defined by <sys/ptrace.h>.
+* The nonstandard siginfo_t fields 'si_int' and 'si_ptr' are deprecated, and
+  will be removed in a future release.  Programs should instead use
+  'si_value.sival_int' and 'si_value.sival_ptr', respectively.
 
 * libm no longer supports SVID error handling (calling a user-provided
   matherr function on error) or the _LIB_VERSION variable to control error
@@ -27,7 +30,18 @@ Deprecated and removed features, and other changes affecting compatibility:
 
 Changes to build and runtime requirements:
 
-  [Add changes to build and runtime requirements here]
+* Some headers now make unconditional use of the C2011 'anonymous union'
+  feature.  In order to compile a program that uses any of these headers,
+  you must use a compiler that supports this feature, even when conforming
+  to an earlier language standard.  This has been true of GCC and Clang for
+  many years.  The structures and headers affected are:
+
+    - siginfo_t (signal.h, sys/wait.h)
+
+  The advantage of this change is that these headers define many fewer
+  macros with surprising expansions (for instance, the identifier 'si_pid'
+  used to be a macro expanding to '_sifields._kill.si_pid' on GNU/Linux
+  systems).
 
 Security related changes:
 
index 33766d1813c689a8942e8f5d37758105a7501036..2d5fbdaf30a463097a809e5e53226d4d3d246923 100644 (file)
@@ -52,38 +52,33 @@ typedef struct
       {
        int _pad[__SI_PAD_SIZE];
 
-        /* kill().  */
+        /* Signals sent by kill or sigqueue.  si_sigval is only valid for
+           sigqueue.  */
        struct
          {
            __pid_t si_pid;     /* Sending process ID.  */
            __uid_t si_uid;     /* Real user ID of sending process.  */
-         } _kill;
+           __sigval_t si_value;        /* Signal value.  */
+         };
 
-       /* POSIX.1b timers.  */
+       /* POSIX.1b timers.  'si_sigval' (above) is also valid.  */
        struct
          {
-           int si_tid;         /* Timer ID.  */
+           int si_timerid;     /* Timer ID.  */
            int si_overrun;     /* Overrun count.  */
            __sigval_t si_sigval;       /* Signal value.  */
-         } _timer;
+         };
 
-       /* POSIX.1b signals.  */
+       /* SIGCHLD.  The first two fields overlay the si_pid and si_uid
+          fields above.  */
        struct
          {
-           __pid_t si_pid;     /* Sending process ID.  */
-           __uid_t si_uid;     /* Real user ID of sending process.  */
-           __sigval_t si_sigval;       /* Signal value.  */
-         } _rt;
-
-       /* SIGCHLD.  */
-       struct
-         {
-           __pid_t si_pid;     /* Which child.  */
-           __uid_t si_uid;     /* Real user ID of sending process.  */
+           __pid_t __sigchld_si_pid;   /* Which child.  */
+           __uid_t __sigchld_si_uid;   /* Real user ID of sending process.  */
            int si_status;      /* Exit value or signal.  */
            __SI_CLOCK_T si_utime;
            __SI_CLOCK_T si_stime;
-         } _sigchld;
+         };
 
        /* SIGILL, SIGFPE, SIGSEGV, SIGBUS.  */
        struct
@@ -96,56 +91,42 @@ typedef struct
                /* used when si_code=SEGV_BNDERR */
                struct
                  {
-                   void *_lower;
-                   void *_upper;
-                 } _addr_bnd;
+                   void *si_lower;
+                   void *si_upper;
+                 };
                /* used when si_code=SEGV_PKUERR */
-               __uint32_t _pkey;
-             } _bounds;
-         } _sigfault;
+               __uint32_t si_pkey;
+             };
+         };
 
        /* SIGPOLL.  */
        struct
          {
            long int si_band;   /* Band event for SIGPOLL.  */
            int si_fd;
-         } _sigpoll;
+         };
 
        /* SIGSYS.  */
 #if __SI_HAVE_SIGSYS
        struct
          {
-           void *_call_addr;   /* Calling user insn.  */
-           int _syscall;       /* Triggering system call number.  */
-           unsigned int _arch; /* AUDIT_ARCH_* of syscall.  */
-         } _sigsys;
+           void *si_call_addr;   /* Calling user insn.  */
+           int si_syscall;       /* Triggering system call number.  */
+           unsigned int si_arch; /* AUDIT_ARCH_* of syscall.  */
+         };
 #endif
-      } _sifields;
+      };
   } siginfo_t __SI_ALIGNMENT;
 
 
-/* X/Open requires some more fields with fixed names.  */
-#define si_pid         _sifields._kill.si_pid
-#define si_uid         _sifields._kill.si_uid
-#define si_timerid     _sifields._timer.si_tid
-#define si_overrun     _sifields._timer.si_overrun
-#define si_status      _sifields._sigchld.si_status
-#define si_utime       _sifields._sigchld.si_utime
-#define si_stime       _sifields._sigchld.si_stime
-#define si_value       _sifields._rt.si_sigval
-#define si_int         _sifields._rt.si_sigval.sival_int
-#define si_ptr         _sifields._rt.si_sigval.sival_ptr
-#define si_addr                _sifields._sigfault.si_addr
-#define si_addr_lsb    _sifields._sigfault.si_addr_lsb
-#define si_lower       _sifields._sigfault._bounds._addr_bnd._lower
-#define si_upper       _sifields._sigfault._bounds._addr_bnd._upper
-#define si_pkey                _sifields._sigfault._bounds._pkey
-#define si_band                _sifields._sigpoll.si_band
-#define si_fd          _sifields._sigpoll.si_fd
-#if __SI_HAVE_SIGSYS
-# define si_call_addr  _sifields._sigsys._call_addr
-# define si_syscall    _sifields._sigsys._syscall
-# define si_arch       _sifields._sigsys._arch
+/* These field aliases are not in POSIX, and are preserved for
+   backward compatibility only.  They may be removed in a future
+   release.  */
+#ifdef __USE_MISC
+#define si_int         si_value.sival_int \
+  __glibc_macro_warning("si_int is deprecated, use si_value.sival_int instead")
+#define si_ptr         si_value.sival_ptr \
+  __glibc_macro_warning("si_ptr is deprecated, use si_value.sival_ptr instead")
 #endif
 
 #endif
index 8b5647062c9cab18d3e146130f63ab06c663f056..a2144c3771288cfd0414f0b06be3790e3e9ab433 100644 (file)
@@ -3,15 +3,16 @@
 
 #define __SI_HAVE_SIGSYS 0
 
-#define __SI_SIGFAULT_ADDL                     \
-  int _si_imm;                                 \
-  unsigned int _si_flags;                      \
-  unsigned long int _si_isr;
-
 #ifdef __USE_GNU
-# define si_imm                _sifields._sigfault._si_imm
-# define si_segvflags  _sifields._sigfault._si_flags
-# define si_isr                _sifields._sigfault._si_isr
+# define __SI_SIGFAULT_ADDL                    \
+  int si_imm;                                  \
+  unsigned int si_segvflags;                   \
+  unsigned long int si_isr;
+#else
+# define __SI_SIGFAULT_ADDL                    \
+  int __si_imm;                                        \
+  unsigned int __si_segvflags;                 \
+  unsigned long int __si_isr;
 #endif
 
 #endif
index 9f79715ebe6c94828bb383be5edee5f2a1ea2007..146fa2c00a35b4a96ae2ed6ef0c592ba836ae23d 100644 (file)
@@ -4,9 +4,10 @@
 
 #define __SI_BAND_TYPE int
 
-#define __SI_SIGFAULT_ADDL \
-  int _si_trapno;
-
-#define si_trapno      _sifields._sigfault._si_trapno
+#ifdef __USE_GNU
+# define __SI_SIGFAULT_ADDL  int si_trapno;
+#else
+# define __SI_SIGFAULT_ADDL  int __si_trapno;
+#endif
 
 #endif
index 7d0c24c84b94bab6c5ebe3ff8e835beae959be27..0421fa358565a72e47c9bcbf3ec54c27471aa1b0 100644 (file)
@@ -2,9 +2,10 @@
 #ifndef _BITS_SIGINFO_ARCH_H
 #define _BITS_SIGINFO_ARCH_H 1
 
-#define __SI_SIGFAULT_ADDL \
-  int _si_trapno;
-
-#define si_trapno      _sifields._sigfault._si_trapno
+#ifdef __USE_GNU
+# define __SI_SIGFAULT_ADDL  int si_trapno;
+#else
+# define __SI_SIGFAULT_ADDL  int __si_trapno;
+#endif
 
 #endif
index 1d813046785b7653d9af478dbd4c48844e24e7c2..696ab450c61e571c91ae86eb5c833ffaec56d44d 100644 (file)
@@ -92,7 +92,7 @@ timer_helper_thread (void *arg)
        {
          if (si.si_code == SI_TIMER)
            {
-             struct timer *tk = (struct timer *) si.si_ptr;
+             struct timer *tk = (struct timer *) si.si_value.sival_ptr;
 
              /* Check the timer is still used and will not go away
                 while we are reading the values here.  */
index 253ebf2e159af7f2637786ca1381d604a013e03a..f24faf35e149bef65597682fdd8115cab950607b 100644 (file)
@@ -95,9 +95,10 @@ do_test (void)
       return 1;
     }
 
-  if (si.si_int != (int) p)
+  if (si.si_value.sival_int != (int) p)
     {
-      printf ("expected PID %d, got si_int %d\n", (int) p, si.si_int);
+      printf ("expected PID %d, got si_int %d\n",
+             (int) p, si.si_value.sival_int);
       kill (p, SIGKILL);
       return 1;
     }