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

[Christian Brauner]

On May 30, 2019 2:29:43 PM GMT+02:00, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>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

I'll pick this patch up.

Acked-by: Christian Brauner <christian@xxxxxxxxxx>