| Message ID | 20190516151249.19029-3-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] sysvipc: Fix compat msgctl | expand |
* Adhemerval Zanella: > diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h > index 65adbb093e..49018c1b28 100644 > --- a/sysdeps/unix/sysv/linux/ipc_priv.h > +++ b/sysdeps/unix/sysv/linux/ipc_priv.h > @@ -34,4 +34,7 @@ struct __old_ipc_perm > #define MSGRCV_ARGS(__msgp, __msgtyp) \ > ((long int []){ (long int) __msgp, __msgtyp }) > > +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ > + (__nsops), 0, (__sops), (__timeout) Maybe add a reference to the s390 version? > diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h b/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h > new file mode 100644 > index 0000000000..9d74e92289 > +/* The s390 sys_ipc variant has only five parameters instead of six > + (as for default variant) and the only difference is the handling of > + SEMTIMEDOP where on s390 the third parameter is used as a pointer > + to a struct timespec where the generic variant uses fifth parameter. */ > +#undef SEMTIMEDOP_IPC_ARGS > +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ > + (__nsops), (__timeout), (__sops) I find “and the only difference is” a bit confusing here. Maybe write “. The difference is” instead? Rest of the patch looks okay to me. Thanks, Florian
On 16/05/2019 12:41, Florian Weimer wrote: > * Adhemerval Zanella: > >> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h >> index 65adbb093e..49018c1b28 100644 >> --- a/sysdeps/unix/sysv/linux/ipc_priv.h >> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h >> @@ -34,4 +34,7 @@ struct __old_ipc_perm >> #define MSGRCV_ARGS(__msgp, __msgtyp) \ >> ((long int []){ (long int) __msgp, __msgtyp }) >> >> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ >> + (__nsops), 0, (__sops), (__timeout) > > Maybe add a reference to the s390 version? What about: /* This macro is required to handle the s390 variants, which passes the arguments in a different order than default. */ > >> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h b/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h >> new file mode 100644 >> index 0000000000..9d74e92289 > >> +/* The s390 sys_ipc variant has only five parameters instead of six >> + (as for default variant) and the only difference is the handling of >> + SEMTIMEDOP where on s390 the third parameter is used as a pointer >> + to a struct timespec where the generic variant uses fifth parameter. */ >> +#undef SEMTIMEDOP_IPC_ARGS >> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ >> + (__nsops), (__timeout), (__sops) > > I find “and the only difference is” a bit confusing here. Maybe write > “. The difference is” instead? Ack, I change it locally. > > Rest of the patch looks okay to me. > > Thanks, > Florian >
* Adhemerval Zanella: > On 16/05/2019 12:41, Florian Weimer wrote: >> * Adhemerval Zanella: >> >>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h >>> index 65adbb093e..49018c1b28 100644 >>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h >>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h >>> @@ -34,4 +34,7 @@ struct __old_ipc_perm >>> #define MSGRCV_ARGS(__msgp, __msgtyp) \ >>> ((long int []){ (long int) __msgp, __msgtyp }) >>> >>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ >>> + (__nsops), 0, (__sops), (__timeout) >> >> Maybe add a reference to the s390 version? > > What about: > > /* This macro is required to handle the s390 variants, which passes the > arguments in a different order than default. */ Isn't this for s390 (31-bit) only? Thanks, Florian
> Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto: > > * Adhemerval Zanella: > >>> On 16/05/2019 12:41, Florian Weimer wrote: >>> * Adhemerval Zanella: >>> >>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h >>>> index 65adbb093e..49018c1b28 100644 >>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h >>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h >>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm >>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \ >>>> ((long int []){ (long int) __msgp, __msgtyp }) >>>> >>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ >>>> + (__nsops), 0, (__sops), (__timeout) >>> >>> Maybe add a reference to the s390 version? >> >> What about: >> >> /* This macro is required to handle the s390 variants, which passes the >> arguments in a different order than default. */ > > Isn't this for s390 (31-bit) only? > Indeed, I will change to “s390-32 variant”. > Thanks, > Florian
* Adhemerval Zanella: >> Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto: >> >> * Adhemerval Zanella: >> >>>> On 16/05/2019 12:41, Florian Weimer wrote: >>>> * Adhemerval Zanella: >>>> >>>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h >>>>> index 65adbb093e..49018c1b28 100644 >>>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h >>>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h >>>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm >>>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \ >>>>> ((long int []){ (long int) __msgp, __msgtyp }) >>>>> >>>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ >>>>> + (__nsops), 0, (__sops), (__timeout) >>>> >>>> Maybe add a reference to the s390 version? >>> >>> What about: >>> >>> /* This macro is required to handle the s390 variants, which passes the >>> arguments in a different order than default. */ >> >> Isn't this for s390 (31-bit) only? >> > > Indeed, I will change to “s390-32 variant”. Thanks, looks good to me. Florian
On Fri, May 17, 2019 at 2:30 PM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto: > > > > * Adhemerval Zanella: > > > >>> On 16/05/2019 12:41, Florian Weimer wrote: > >>> * Adhemerval Zanella: > >>> > >>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h > >>>> index 65adbb093e..49018c1b28 100644 > >>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h > >>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h > >>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm > >>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \ > >>>> ((long int []){ (long int) __msgp, __msgtyp }) > >>>> > >>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ > >>>> + (__nsops), 0, (__sops), (__timeout) > >>> > >>> Maybe add a reference to the s390 version? > >> > >> What about: > >> > >> /* This macro is required to handle the s390 variants, which passes the > >> arguments in a different order than default. */ > > > > Isn't this for s390 (31-bit) only? > > > > Indeed, I will change to “s390-32 variant”. According to the kernel sources, the odd sys_s390_ipc function is used in both native 64-bit and compat 32-bit variants. The 64-bit s390 ABI also started out with a maximum of five arguments, so the wrapper was kept there, see: arch/s390/kernel/syscalls/syscall.tbl:117 common ipc sys_s390_ipc compat_sys_s390_ipc Arnd
On 17/05/2019 16:09, Arnd Bergmann wrote: > On Fri, May 17, 2019 at 2:30 PM Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >>> Il giorno 17 mag 2019, alle ore 06:13, Florian Weimer <fweimer@redhat.com> ha scritto: >>> >>> * Adhemerval Zanella: >>> >>>>> On 16/05/2019 12:41, Florian Weimer wrote: >>>>> * Adhemerval Zanella: >>>>> >>>>>> diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h >>>>>> index 65adbb093e..49018c1b28 100644 >>>>>> --- a/sysdeps/unix/sysv/linux/ipc_priv.h >>>>>> +++ b/sysdeps/unix/sysv/linux/ipc_priv.h >>>>>> @@ -34,4 +34,7 @@ struct __old_ipc_perm >>>>>> #define MSGRCV_ARGS(__msgp, __msgtyp) \ >>>>>> ((long int []){ (long int) __msgp, __msgtyp }) >>>>>> >>>>>> +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ >>>>>> + (__nsops), 0, (__sops), (__timeout) >>>>> >>>>> Maybe add a reference to the s390 version? >>>> >>>> What about: >>>> >>>> /* This macro is required to handle the s390 variants, which passes the >>>> arguments in a different order than default. */ >>> >>> Isn't this for s390 (31-bit) only? >>> >> >> Indeed, I will change to “s390-32 variant”. > > According to the kernel sources, the odd sys_s390_ipc function is > used in both native 64-bit and compat 32-bit variants. The 64-bit > s390 ABI also started out with a maximum of five arguments, > so the wrapper was kept there, see: > > arch/s390/kernel/syscalls/syscall.tbl:117 common ipc > sys_s390_ipc compat_sys_s390_ipc > Thanks for remind me this is not for 31-bit only. I moved the sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h to sysdeps/unix/sysv/linux/s390/ipc_priv.h.
diff --git a/sysdeps/unix/sysv/linux/ipc_priv.h b/sysdeps/unix/sysv/linux/ipc_priv.h index 65adbb093e..49018c1b28 100644 --- a/sysdeps/unix/sysv/linux/ipc_priv.h +++ b/sysdeps/unix/sysv/linux/ipc_priv.h @@ -34,4 +34,7 @@ struct __old_ipc_perm #define MSGRCV_ARGS(__msgp, __msgtyp) \ ((long int []){ (long int) __msgp, __msgtyp }) +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ + (__nsops), 0, (__sops), (__timeout) + #include <ipc_ops.h> diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h b/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h new file mode 100644 index 0000000000..9d74e92289 --- /dev/null +++ b/sysdeps/unix/sysv/linux/s390/s390-32/ipc_priv.h @@ -0,0 +1,29 @@ +/* Arch-specific SysV IPC definitions for Linux. s390 version. + Copyright (C) 2019 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + 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/>. */ + +#include <sysdeps/unix/sysv/linux/ipc_priv.h> + +/* The s390 sys_ipc variant has only five parameters instead of six + (as for default variant) and the only difference is the handling of + SEMTIMEDOP where on s390 the third parameter is used as a pointer + to a struct timespec where the generic variant uses fifth parameter. */ +#undef SEMTIMEDOP_IPC_ARGS +#define SEMTIMEDOP_IPC_ARGS(__nsops, __sops, __timeout) \ + (__nsops), (__timeout), (__sops) + +#include <ipc_ops.h> diff --git a/sysdeps/unix/sysv/linux/s390/semtimedop.c b/sysdeps/unix/sysv/linux/s390/semtimedop.c deleted file mode 100644 index 772ee432a9..0000000000 --- a/sysdeps/unix/sysv/linux/s390/semtimedop.c +++ /dev/null @@ -1,36 +0,0 @@ -/* Copyright (C) 2003-2019 Free Software Foundation, Inc. - This file is part of the GNU C Library. - Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - 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/>. */ - -#include <sys/sem.h> -#include <ipc_priv.h> -#include <sysdep.h> -#include <errno.h> - -/* Perform user-defined atomical operation of array of semaphores. */ - -int -semtimedop (int semid, struct sembuf *sops, size_t nsops, - const struct timespec *timeout) -{ - /* The s390 sys_ipc variant has only five parameters instead of six - (as for default variant) and the only difference is the handling of - SEMTIMEDOP where on s390 the third parameter is used as a pointer - to a struct timespec where the generic variant uses fifth parameter. */ - return INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, nsops, timeout, - sops); -} diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c index 961c6d1f0b..1d746c4117 100644 --- a/sysdeps/unix/sysv/linux/semtimedop.c +++ b/sysdeps/unix/sysv/linux/semtimedop.c @@ -30,7 +30,7 @@ semtimedop (int semid, struct sembuf *sops, size_t nsops, #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS return INLINE_SYSCALL_CALL (semtimedop, semid, sops, nsops, timeout); #else - return INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, nsops, 0, sops, - timeout); + return INLINE_SYSCALL_CALL (ipc, IPCOP_semtimedop, semid, + SEMTIMEDOP_IPC_ARGS (nsops, sops, timeout)); #endif }