Re: [PATCH v3 3/3] sysv: linux: Pass 64-bit version of semctl syscall

[Alistair Francis <alistair23 at gmail dot com>]

On Thu, Mar 5, 2020 at 2:14 AM Andreas Schwab <schwab@suse.de> wrote:
>
> On Mär 04 2020, Alistair Francis wrote:
>
> > We protect this new code via the __IPC_TIME64 marco, which is only true
> > for 32-bit architectures with a 64-bit time_t.
>
> There is only a single use of the macro, so I don't see much point in
> it.

This came from comments from Adhemerval Zanella on the previous
version which you can see here:
https://patchwork.ozlabs.org/patch/1233625/#2367606

>
> > diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> > index 0c3eb0932f..3ac6d01b84 100644
> > --- a/sysdeps/unix/sysv/linux/semctl.c
> > +++ b/sysdeps/unix/sysv/linux/semctl.c
> > @@ -23,11 +23,16 @@
> >  #include <shlib-compat.h>
> >  #include <errno.h>
> >
> > +#define __IPC_TIME64 (IPC_STAT & __IPC_64)
> > +
> >  /* Define a `union semun' suitable for Linux here.  */
> >  union semun
> >  {
> >    int val;                   /* value for SETVAL */
> >    struct semid_ds *buf;              /* buffer for IPC_STAT & IPC_SET */
> > +#if __WORDSIZE == 32
> > +  struct __semid_ds32 *buf32;   /* 32-bit buffer for IPC_STAT & IPC_SET */
>
> Currently, this is only used for IPC_STAT.

I'll update the comment.

>
> > +#endif
> >    unsigned short int *array; /* array for GETALL & SETALL */
> >    struct seminfo *__buf;     /* buffer for IPC_INFO */
> >  };
> > @@ -43,13 +48,25 @@ union semun
> >  static int
> >  semctl_syscall (int semid, int semnum, int cmd, union semun arg)
> >  {
> > +  int ret;
> >  #ifdef __ASSUME_DIRECT_SYSVIPC_SYSCALLS
> > -  return INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > -                           arg.array);
> > +  ret = INLINE_SYSCALL_CALL (semctl, semid, semnum, cmd | __IPC_64,
> > +                             arg.array);
> >  #else
> > -  return INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64,
> > -                           SEMCTL_ARG_ADDRESS (arg));
> > +  ret = INLINE_SYSCALL_CALL (ipc, IPCOP_semctl, semid, semnum, cmd | __IPC_64,
> > +                             SEMCTL_ARG_ADDRESS (arg));
> > +#endif
> > +
> > +#if __IPC_TIME64
> > +  if (ret == 0 && (cmd & __IPC_TIME64))
>
> I don't think that's the right condition.  IIUC this should check for
> cmd == IPC_STAT.

I'm not sure, I'll check to see what it should be.

Alistair

>
> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."