Re: [PATCH] samples: pidfd: Fix compile error seen if __NR_pidfd_send_signal is undefined

[Guenter Roeck]

On 5/30/19 4:55 AM, Christian Brauner wrote:
> On May 30, 2019 1:50:31 PM GMT+02:00, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > On 5/30/19 4:43 AM, Christian Brauner wrote:
> > > On May 30, 2019 1:40:47 PM GMT+02:00, Guenter Roeck
> > <linux@xxxxxxxxxxxx> wrote:
> > > > To make pidfd-metadata compile on all arches, irrespective of
> > whether
> > > > or not syscall numbers are assigned, define the syscall number to -1
> > > > if it isn't to cause the kernel to return -ENOSYS.
> > > >
> > > > Fixes: 43c6afee48d4 ("samples: show race-free pidfd metadata
> > access")
> > > > Cc: Christian Brauner <christian@xxxxxxxxxx>
> > > > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > > > ---
> > > > samples/pidfd/pidfd-metadata.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/samples/pidfd/pidfd-metadata.c
> > > > b/samples/pidfd/pidfd-metadata.c
> > > > index 640f5f757c57..1e125ddde268 100644
> > > > --- a/samples/pidfd/pidfd-metadata.c
> > > > +++ b/samples/pidfd/pidfd-metadata.c
> > > > @@ -21,6 +21,10 @@
> > > > #define CLONE_PIDFD 0x00001000
> > > > #endif
> > > >
> > > > +#ifndef __NR_pidfd_send_signal
> > > > +#define __NR_pidfd_send_signal	-1
> > > > +#endif
> > > > +
> > > > static int do_child(void *args)
> > > > {
> > > > 	printf("%d\n", getpid());
> > >
> > > Couldn't you just use the actual syscall number?
> > > That should still fail if the kernel is to old
> > > and still work on kernels that support it
> > > but for whatever reason the unistd.h h
> > > header doesn't have it defined.
> >
> > syscall numbers can differ from architecture to architecture, and the
> > provided solution is used in other test code. Please feel free to
> > submit
> > a different patch, though - I am only interested in a fix, which
> > doesn't
> > have to be mine.
> >
> > Note that this fails in mips builds.
> >
> > Thanks,
> > Guenter
>
> The syscall number is the same on all arches for this syscall.
> It's been added after the syscall numbering
> work by Arnd.

As I suggested, please feel free to submit a different patch to fix the problem.
What you are saying may be correct, but I would not personally want to rely on it
or create a hard assumption that it will always be the case.

Thanks,
Guenter