| Message ID | 1518439345-6013-1-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/4] Rename nptl-signals.h to internal-signals.h | expand |
Ping. On 12/02/2018 10:42, Adhemerval Zanella wrote: > Changes from previous version: > > - Fixed documentation issues pointed out by Rical Jasan. > > -- > > This patch renames the nptl-signals.h header to internal-signals.h. > On Linux the definitions and functions are not only NPTL related, but > used for other POSIX definitions as well (for instance SIGTIMER for > posix times, SIGSETXID for id functions, and signal block/restore > helpers) and since generic functions will be places and used in generic > implementation it makes more sense to decouple it from NPTL. > > Checked on x86_64-linux-gnu. > > * sysdeps/nptl/nptl-signals.h: Move to ... > * sysdeps/generic/internal-signals.h: ... here. Adjust internal > comments. > * sysdeps/unix/sysv/linux/internal-signals.h: Add include guards. > (__nptl_is_internal_signal): Rename to __is_internal_signal. > (__nptl_clear_internal_signals): Rename to __clear_internal_signals. > * sysdeps/unix/sysv/linux/raise.c: Adjust nptl-signal.h to > include-signals.h rename. > * nptl/pthreadP.h: Likewise. > * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Call > __is_internal_signal instead of __nptl_is_internal_signal. > > Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > ChangeLog | 14 ++++++++++++++ > nptl/pthreadP.h | 2 +- > .../{nptl/nptl-signals.h => generic/internal-signals.h} | 7 +------ > .../sysv/linux/{nptl-signals.h => internal-signals.h} | 16 ++++++++++------ > sysdeps/unix/sysv/linux/raise.c | 2 +- > sysdeps/unix/sysv/linux/spawni.c | 2 +- > 6 files changed, 28 insertions(+), 15 deletions(-) > rename sysdeps/{nptl/nptl-signals.h => generic/internal-signals.h} (74%) > rename sysdeps/unix/sysv/linux/{nptl-signals.h => internal-signals.h} (89%) > > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > index 583515f..075530c 100644 > --- a/nptl/pthreadP.h > +++ b/nptl/pthreadP.h > @@ -32,7 +32,7 @@ > #include <atomic.h> > #include <kernel-features.h> > #include <errno.h> > -#include <nptl-signals.h> > +#include <internal-signals.h> > > > /* Atomic operations on TLS memory. */ > diff --git a/sysdeps/nptl/nptl-signals.h b/sysdeps/generic/internal-signals.h > similarity index 74% > rename from sysdeps/nptl/nptl-signals.h > rename to sysdeps/generic/internal-signals.h > index e1275c7..01e5b75 100644 > --- a/sysdeps/nptl/nptl-signals.h > +++ b/sysdeps/generic/internal-signals.h > @@ -1,4 +1,4 @@ > -/* Special use of signals in NPTL internals. Stub version. > +/* Special use of signals internally. Stub version. > Copyright (C) 2014-2018 Free Software Foundation, Inc. > This file is part of the GNU C Library. > > @@ -15,8 +15,3 @@ > You should have received a copy of the GNU Lesser General Public > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > - > -/* This file can define the macros SIGCANCEL, SIGTIMER, and SIGSETXID to > - signal numbers reserved by libpthread for those internal purposes. > - > - Note that some code presumes SIGTIMER is the same as SIGCANCEL. */ > diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h > similarity index 89% > rename from sysdeps/unix/sysv/linux/nptl-signals.h > rename to sysdeps/unix/sysv/linux/internal-signals.h > index e789198..e007372 100644 > --- a/sysdeps/unix/sysv/linux/nptl-signals.h > +++ b/sysdeps/unix/sysv/linux/internal-signals.h > @@ -1,4 +1,4 @@ > -/* Special use of signals in NPTL internals. Linux version. > +/* Special use of signals internally. Linux version. > Copyright (C) 2014-2018 Free Software Foundation, Inc. > This file is part of the GNU C Library. > > @@ -16,6 +16,9 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > +#ifndef __INTERNAL_SIGNALS_H > +# define __INTERNAL_SIGNALS_H > + > #include <signal.h> > #include <sigsetops.h> > > @@ -35,17 +38,16 @@ > > /* Return is sig is used internally. */ > static inline int > -__nptl_is_internal_signal (int sig) > +__is_internal_signal (int sig) > { > - return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID); > + return (sig == SIGCANCEL) || (sig == SIGSETXID); > } > > /* Remove internal glibc signal from the mask. */ > static inline void > -__nptl_clear_internal_signals (sigset_t *set) > +__clear_internal_signals (sigset_t *set) > { > __sigdelset (set, SIGCANCEL); > - __sigdelset (set, SIGTIMER); > __sigdelset (set, SIGSETXID); > } > > @@ -66,7 +68,7 @@ static inline int > __libc_signal_block_app (sigset_t *set) > { > sigset_t allset = SIGALL_SET; > - __nptl_clear_internal_signals (&allset); > + __clear_internal_signals (&allset); > INTERNAL_SYSCALL_DECL (err); > return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set, > _NSIG / 8); > @@ -83,3 +85,5 @@ __libc_signal_restore_set (const sigset_t *set) > > /* Used to communicate with signal handler. */ > extern struct xid_command *__xidcmd attribute_hidden; > + > +#endif > diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c > index cb98f90..b05eae2 100644 > --- a/sysdeps/unix/sysv/linux/raise.c > +++ b/sysdeps/unix/sysv/linux/raise.c > @@ -21,7 +21,7 @@ > #include <errno.h> > #include <sys/types.h> > #include <unistd.h> > -#include <nptl-signals.h> > +#include <internal-signals.h> > > int > raise (int sig) > diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c > index 6b699a4..0391b9b 100644 > --- a/sysdeps/unix/sysv/linux/spawni.c > +++ b/sysdeps/unix/sysv/linux/spawni.c > @@ -144,7 +144,7 @@ __spawni_child (void *arguments) > } > else if (sigismember (&hset, sig)) > { > - if (__nptl_is_internal_signal (sig)) > + if (__is_internal_signal (sig)) > sa.sa_handler = SIG_IGN; > else > { >
On 02/12/2018 01:42 PM, Adhemerval Zanella wrote: > This patch renames the nptl-signals.h header to internal-signals.h. > On Linux the definitions and functions are not only NPTL related, but > used for other POSIX definitions as well (for instance SIGTIMER for > posix times, SIGSETXID for id functions, and signal block/restore POSIX timers? > Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> We don't use DCO, but have copyright assignments. > diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h > diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h > similarity index 89% > rename from sysdeps/unix/sysv/linux/nptl-signals.h > rename to sysdeps/unix/sysv/linux/internal-signals.h > index e789198..e007372 100644 > --- a/sysdeps/unix/sysv/linux/nptl-signals.h > +++ b/sysdeps/unix/sysv/linux/internal-signals.h > @@ -1,4 +1,4 @@ > -/* Special use of signals in NPTL internals. Linux version. > +/* Special use of signals internally. Linux version. > Copyright (C) 2014-2018 Free Software Foundation, Inc. > This file is part of the GNU C Library. > > @@ -16,6 +16,9 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > +#ifndef __INTERNAL_SIGNALS_H > +# define __INTERNAL_SIGNALS_H > + > #include <signal.h> > #include <sigsetops.h> > > @@ -35,17 +38,16 @@ > > /* Return is sig is used internally. */ > static inline int > -__nptl_is_internal_signal (int sig) > +__is_internal_signal (int sig) > { > - return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID); > + return (sig == SIGCANCEL) || (sig == SIGSETXID); Should this change be mentioned in the ChangeLog? You could remove the unnecessary parens because you modify this line anyway. > /* Remove internal glibc signal from the mask. */ > static inline void > -__nptl_clear_internal_signals (sigset_t *set) > +__clear_internal_signals (sigset_t *set) > { > __sigdelset (set, SIGCANCEL); > - __sigdelset (set, SIGTIMER); Likewise, should be mentioned in the ChangeLog entry. Looks okay otherwise. Thanks, Florian
On 20/02/2018 09:32, Florian Weimer wrote: > On 02/12/2018 01:42 PM, Adhemerval Zanella wrote: >> This patch renames the nptl-signals.h header to internal-signals.h. >> On Linux the definitions and functions are not only NPTL related, but >> used for other POSIX definitions as well (for instance SIGTIMER for >> posix times, SIGSETXID for id functions, and signal block/restore > > POSIX timers? > >> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > We don't use DCO, but have copyright assignments. Right, although I had the impression signed-off-by is mostly optional. I am trying to find the follow-up thread about it after it was brought up in Cauldron. Do you know if Carlos has written some wiki entry about it? > >> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h >> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h >> similarity index 89% >> rename from sysdeps/unix/sysv/linux/nptl-signals.h >> rename to sysdeps/unix/sysv/linux/internal-signals.h >> index e789198..e007372 100644 >> --- a/sysdeps/unix/sysv/linux/nptl-signals.h >> +++ b/sysdeps/unix/sysv/linux/internal-signals.h >> @@ -1,4 +1,4 @@ >> -/* Special use of signals in NPTL internals. Linux version. >> +/* Special use of signals internally. Linux version. >> Copyright (C) 2014-2018 Free Software Foundation, Inc. >> This file is part of the GNU C Library. >> @@ -16,6 +16,9 @@ >> License along with the GNU C Library; if not, see >> <http://www.gnu.org/licenses/>. */ >> +#ifndef __INTERNAL_SIGNALS_H >> +# define __INTERNAL_SIGNALS_H >> + >> #include <signal.h> >> #include <sigsetops.h> >> @@ -35,17 +38,16 @@ >> /* Return is sig is used internally. */ >> static inline int >> -__nptl_is_internal_signal (int sig) >> +__is_internal_signal (int sig) >> { >> - return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID); >> + return (sig == SIGCANCEL) || (sig == SIGSETXID); > > Should this change be mentioned in the ChangeLog? You could remove the unnecessary parens because you modify this line anyway. I will add a entry in ChangeLog and remove the unnecessary parentheses. > >> /* Remove internal glibc signal from the mask. */ >> static inline void >> -__nptl_clear_internal_signals (sigset_t *set) >> +__clear_internal_signals (sigset_t *set) >> { >> __sigdelset (set, SIGCANCEL); >> - __sigdelset (set, SIGTIMER); > > Likewise, should be mentioned in the ChangeLog entry. I will a entry as well. > > Looks okay otherwise. > > Thanks, > Florian
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index 583515f..075530c 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -32,7 +32,7 @@ #include <atomic.h> #include <kernel-features.h> #include <errno.h> -#include <nptl-signals.h> +#include <internal-signals.h> /* Atomic operations on TLS memory. */ diff --git a/sysdeps/nptl/nptl-signals.h b/sysdeps/generic/internal-signals.h similarity index 74% rename from sysdeps/nptl/nptl-signals.h rename to sysdeps/generic/internal-signals.h index e1275c7..01e5b75 100644 --- a/sysdeps/nptl/nptl-signals.h +++ b/sysdeps/generic/internal-signals.h @@ -1,4 +1,4 @@ -/* Special use of signals in NPTL internals. Stub version. +/* Special use of signals internally. Stub version. Copyright (C) 2014-2018 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -15,8 +15,3 @@ You should have received a copy of the GNU Lesser General Public License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ - -/* This file can define the macros SIGCANCEL, SIGTIMER, and SIGSETXID to - signal numbers reserved by libpthread for those internal purposes. - - Note that some code presumes SIGTIMER is the same as SIGCANCEL. */ diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h similarity index 89% rename from sysdeps/unix/sysv/linux/nptl-signals.h rename to sysdeps/unix/sysv/linux/internal-signals.h index e789198..e007372 100644 --- a/sysdeps/unix/sysv/linux/nptl-signals.h +++ b/sysdeps/unix/sysv/linux/internal-signals.h @@ -1,4 +1,4 @@ -/* Special use of signals in NPTL internals. Linux version. +/* Special use of signals internally. Linux version. Copyright (C) 2014-2018 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -16,6 +16,9 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#ifndef __INTERNAL_SIGNALS_H +# define __INTERNAL_SIGNALS_H + #include <signal.h> #include <sigsetops.h> @@ -35,17 +38,16 @@ /* Return is sig is used internally. */ static inline int -__nptl_is_internal_signal (int sig) +__is_internal_signal (int sig) { - return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID); + return (sig == SIGCANCEL) || (sig == SIGSETXID); } /* Remove internal glibc signal from the mask. */ static inline void -__nptl_clear_internal_signals (sigset_t *set) +__clear_internal_signals (sigset_t *set) { __sigdelset (set, SIGCANCEL); - __sigdelset (set, SIGTIMER); __sigdelset (set, SIGSETXID); } @@ -66,7 +68,7 @@ static inline int __libc_signal_block_app (sigset_t *set) { sigset_t allset = SIGALL_SET; - __nptl_clear_internal_signals (&allset); + __clear_internal_signals (&allset); INTERNAL_SYSCALL_DECL (err); return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set, _NSIG / 8); @@ -83,3 +85,5 @@ __libc_signal_restore_set (const sigset_t *set) /* Used to communicate with signal handler. */ extern struct xid_command *__xidcmd attribute_hidden; + +#endif diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c index cb98f90..b05eae2 100644 --- a/sysdeps/unix/sysv/linux/raise.c +++ b/sysdeps/unix/sysv/linux/raise.c @@ -21,7 +21,7 @@ #include <errno.h> #include <sys/types.h> #include <unistd.h> -#include <nptl-signals.h> +#include <internal-signals.h> int raise (int sig) diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index 6b699a4..0391b9b 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -144,7 +144,7 @@ __spawni_child (void *arguments) } else if (sigismember (&hset, sig)) { - if (__nptl_is_internal_signal (sig)) + if (__is_internal_signal (sig)) sa.sa_handler = SIG_IGN; else {
Changes from previous version: - Fixed documentation issues pointed out by Rical Jasan. -- This patch renames the nptl-signals.h header to internal-signals.h. On Linux the definitions and functions are not only NPTL related, but used for other POSIX definitions as well (for instance SIGTIMER for posix times, SIGSETXID for id functions, and signal block/restore helpers) and since generic functions will be places and used in generic implementation it makes more sense to decouple it from NPTL. Checked on x86_64-linux-gnu. * sysdeps/nptl/nptl-signals.h: Move to ... * sysdeps/generic/internal-signals.h: ... here. Adjust internal comments. * sysdeps/unix/sysv/linux/internal-signals.h: Add include guards. (__nptl_is_internal_signal): Rename to __is_internal_signal. (__nptl_clear_internal_signals): Rename to __clear_internal_signals. * sysdeps/unix/sysv/linux/raise.c: Adjust nptl-signal.h to include-signals.h rename. * nptl/pthreadP.h: Likewise. * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Call __is_internal_signal instead of __nptl_is_internal_signal. Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> --- ChangeLog | 14 ++++++++++++++ nptl/pthreadP.h | 2 +- .../{nptl/nptl-signals.h => generic/internal-signals.h} | 7 +------ .../sysv/linux/{nptl-signals.h => internal-signals.h} | 16 ++++++++++------ sysdeps/unix/sysv/linux/raise.c | 2 +- sysdeps/unix/sysv/linux/spawni.c | 2 +- 6 files changed, 28 insertions(+), 15 deletions(-) rename sysdeps/{nptl/nptl-signals.h => generic/internal-signals.h} (74%) rename sysdeps/unix/sysv/linux/{nptl-signals.h => internal-signals.h} (89%) -- 2.7.4