| Message ID | 1398781051-16207-6-git-send-email-ncopa@alpinelinux.org |
|---|---|
| State | New |
| Headers | show |
On 04/29/2014 08:17 AM, Natanael Copa wrote: > The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available > on all platforms, so we define those if they are missing. > > This is needed for musl libc. > > Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> > --- > linux-user/signal.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 7d6246f..6019dbb 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -32,6 +32,13 @@ > > //#define DEBUG_SIGNAL > > +#ifndef __SIGRTMIN > +#define __SIGRTMIN 32 Rather than defining the implementation-specific __SIGRTMIN to a magic number that is liable to be wrong, why not instead fix the code to use the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead?
On Tue, 29 Apr 2014 08:28:29 -0600 Eric Blake <eblake@redhat.com> wrote: > On 04/29/2014 08:17 AM, Natanael Copa wrote: > > The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available > > on all platforms, so we define those if they are missing. > > > > This is needed for musl libc. > > > > Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> > > --- > > linux-user/signal.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/linux-user/signal.c b/linux-user/signal.c > > index 7d6246f..6019dbb 100644 > > --- a/linux-user/signal.c > > +++ b/linux-user/signal.c > > @@ -32,6 +32,13 @@ > > > > //#define DEBUG_SIGNAL > > > > +#ifndef __SIGRTMIN > > +#define __SIGRTMIN 32 > > Rather than defining the implementation-specific __SIGRTMIN to a magic > number that is liable to be wrong, why not instead fix the code to use > the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead? > Those seems to be runtime values: /usr/include/signal.h:#define SIGRTMIN (__libc_current_sigrtmin()) /usr/include/signal.h:#define SIGRTMAX (__libc_current_sigrtmax()) so it gives: /home/ncopa/src/qemu/linux-user/signal.c:93:5: error: nonconstant array index in initializer [SIGRTMIN] = __SIGRTMAX, I could have used (NSIG-1) but are not sure if NSIG is a runtime macro in glibc. The array itself is using _NSIG instead of NSIG for some reason. -nc
On 04/29/2014 08:53 AM, Natanael Copa wrote: > On Tue, 29 Apr 2014 08:28:29 -0600 > Eric Blake <eblake@redhat.com> wrote: > >> On 04/29/2014 08:17 AM, Natanael Copa wrote: >>> The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available >>> on all platforms, so we define those if they are missing. >>> +#define __SIGRTMIN 32 >> >> Rather than defining the implementation-specific __SIGRTMIN to a magic >> number that is liable to be wrong, why not instead fix the code to use >> the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead? >> > > Those seems to be runtime values: > /usr/include/signal.h:#define SIGRTMIN (__libc_current_sigrtmin()) Oh right - POSIX allows them to be runtime variable. But we are interacting with a given kernel, where the values will be fixed. Maybe you have to define __SIGRTMIN after all, but can we at least have an assert() that the value you picked matches SIGRTMIN at runtime? > /usr/include/signal.h:#define SIGRTMAX (__libc_current_sigrtmax()) > > so it gives: > /home/ncopa/src/qemu/linux-user/signal.c:93:5: error: nonconstant array index in initializer > [SIGRTMIN] = __SIGRTMAX, > > I could have used (NSIG-1) but are not sure if NSIG is a runtime macro > in glibc. The array itself is using _NSIG instead of NSIG for some > reason. NSIG is not any more portable; nor does POSIX require that the RT signals occur at the tail end of NSIG (in other words, NSIG-1 need not be SIGRTMAX). Unless someone knows of a kernel define, it sounds like we're stuck hard-coding in some knowledge of Linux.
On Tue, 29 Apr 2014 09:02:13 -0600 Eric Blake <eblake@redhat.com> wrote: > On 04/29/2014 08:53 AM, Natanael Copa wrote: > > On Tue, 29 Apr 2014 08:28:29 -0600 > > Eric Blake <eblake@redhat.com> wrote: > > > >> On 04/29/2014 08:17 AM, Natanael Copa wrote: > >>> The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available > >>> on all platforms, so we define those if they are missing. > > >>> +#define __SIGRTMIN 32 > >> > >> Rather than defining the implementation-specific __SIGRTMIN to a magic > >> number that is liable to be wrong, why not instead fix the code to use > >> the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead? > >> > > > > Those seems to be runtime values: > > /usr/include/signal.h:#define SIGRTMIN (__libc_current_sigrtmin()) > > Oh right - POSIX allows them to be runtime variable. But we are > interacting with a given kernel, where the values will be fixed. Maybe > you have to define __SIGRTMIN after all, but can we at least have an > assert() that the value you picked matches SIGRTMIN at runtime? Yeah, that might be an idea. > > /usr/include/signal.h:#define SIGRTMAX (__libc_current_sigrtmax()) > > > > so it gives: > > /home/ncopa/src/qemu/linux-user/signal.c:93:5: error: nonconstant array index in initializer > > [SIGRTMIN] = __SIGRTMAX, > > > > I could have used (NSIG-1) but are not sure if NSIG is a runtime macro > > in glibc. The array itself is using _NSIG instead of NSIG for some > > reason. > > NSIG is not any more portable; nor does POSIX require that the RT > signals occur at the tail end of NSIG (in other words, NSIG-1 need not > be SIGRTMAX). Unless someone knows of a kernel define, it sounds like > we're stuck hard-coding in some knowledge of Linux. > Since we already use _NSIG to define the size of the array, and we want to use the last element of the array, maybe we should just use _NSIG-1? -nc
On Tue, Apr 29, 2014 at 10:06:31PM +0200, Natanael Copa wrote: > On Tue, 29 Apr 2014 09:02:13 -0600 > Eric Blake <eblake@redhat.com> wrote: > > > On 04/29/2014 08:53 AM, Natanael Copa wrote: > > > On Tue, 29 Apr 2014 08:28:29 -0600 > > > Eric Blake <eblake@redhat.com> wrote: > > > > > >> On 04/29/2014 08:17 AM, Natanael Copa wrote: > > >>> The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available > > >>> on all platforms, so we define those if they are missing. > > > > >>> +#define __SIGRTMIN 32 > > >> > > >> Rather than defining the implementation-specific __SIGRTMIN to a magic > > >> number that is liable to be wrong, why not instead fix the code to use > > >> the POSIX-mandated SIGRTMIN and SIGRTMAX public defines instead? > > >> > > > > > > Those seems to be runtime values: > > > /usr/include/signal.h:#define SIGRTMIN (__libc_current_sigrtmin()) > > > > Oh right - POSIX allows them to be runtime variable. But we are > > interacting with a given kernel, where the values will be fixed. Maybe > > you have to define __SIGRTMIN after all, but can we at least have an > > assert() that the value you picked matches SIGRTMIN at runtime? > Yeah, that might be an idea. If you can update the patch with asserts, I'd be fine with the patch. I don't we want to go into changing to use NSIG-1 unless there is some compelling advantage on it. > > > /usr/include/signal.h:#define SIGRTMAX (__libc_current_sigrtmax()) > > > > > > so it gives: > > > /home/ncopa/src/qemu/linux-user/signal.c:93:5: error: nonconstant array index in initializer > > > [SIGRTMIN] = __SIGRTMAX, > > > > > > I could have used (NSIG-1) but are not sure if NSIG is a runtime macro > > > in glibc. The array itself is using _NSIG instead of NSIG for some > > > reason. > > > > NSIG is not any more portable; nor does POSIX require that the RT > > signals occur at the tail end of NSIG (in other words, NSIG-1 need not > > be SIGRTMAX). Unless someone knows of a kernel define, it sounds like > > we're stuck hard-coding in some knowledge of Linux. > > > > Since we already use _NSIG to define the size of the array, and we want > to use the last element of the array, maybe we should just use _NSIG-1? > > -nc
diff --git a/linux-user/signal.c b/linux-user/signal.c index 7d6246f..6019dbb 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -32,6 +32,13 @@ //#define DEBUG_SIGNAL +#ifndef __SIGRTMIN +#define __SIGRTMIN 32 +#endif +#ifndef __SIGRTMAX +#define __SIGRTMAX (NSIG-1) +#endif + static struct target_sigaltstack target_sigaltstack_used = { .ss_sp = 0, .ss_size = 0,
The __SIGRTMIN and __SIGRTMAX are glibc internals and are not available on all platforms, so we define those if they are missing. This is needed for musl libc. Signed-off-by: Natanael Copa <ncopa@alpinelinux.org> --- linux-user/signal.c | 7 +++++++ 1 file changed, 7 insertions(+)