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

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

[Guenter Roeck]

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@brauner.io>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 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());
-- 
2.7.4

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

[Christian Brauner]

On May 30, 2019 1:40:47 PM GMT+02:00, Guenter Roeck <linux@roeck-us.net> 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@brauner.io>
>Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>---
> 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.

Thanks!
Christian

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

[Guenter Roeck]

On 5/30/19 4:43 AM, Christian Brauner wrote:
> On May 30, 2019 1:40:47 PM GMT+02:00, Guenter Roeck <linux@roeck-us.net> 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@brauner.io>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> 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

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

[Christian Brauner]

On May 30, 2019 1:50:31 PM GMT+02:00, Guenter Roeck <linux@roeck-us.net> 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@roeck-us.net> 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@brauner.io>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> 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.

Christian

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@roeck-us.net> 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@roeck-us.net> 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@brauner.io>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> ---
>>>> 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

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@roeck-us.net> 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@roeck-us.net> 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@roeck-us.net> 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@brauner.io>
>>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>>> ---
>>>>> 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@brauner.io>