Re: [PATCH 1/3] ipc: Refactor sysvipc internal definitions

[Florian Weimer <fweimer at redhat dot com>]

* Adhemerval Zanella:

> This patch refactor the internal sysvipc in two main points:
>
>   1. Add a new __ASSUME_SYSVIPC_DEFAULT_IPC_64 to infer the __IPC_64
>      value in wire-up or ipc syscall.  The defaut value assumed for

Sorry, would you please re-phrase?  I don't understand the wire-up
terminology in this context.

>      __IPC_64 is also changed from 0x100 to 0x0, aligning with Linux
>      generic UAPI.  The idea is to simplify the Linux 5.1 wire-up for
>      sysvipc syscalls for some 32-bit ABIs (which expectes __IPC_64
>      being 0x0) and simplify new ports (which would not require add
>      a ipc_priv.h to override the __IPC_64 value).

Replace “would not required add a ipc_priv.h” with “will no longer need
to add ipc_priv.h”?


>   2. It removes some duplicated definition from sysvipc compat code
>      at ipc_priv.h.  The idea is also to make it simpler to enable
>      the new wireup sysvipc on Linux v5.1.

Again, I don't understand this terminology.

> diff --git a/sysdeps/unix/sysv/linux/alpha/kernel-features.h b/sysdeps/unix/sysv/linux/alpha/kernel-features.h
> index 3928b4b62c..ac127adff7 100644
> --- a/sysdeps/unix/sysv/linux/alpha/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/alpha/kernel-features.h
> @@ -52,4 +52,7 @@
>  # undef __ASSUME_STATX
>  #endif
>  
> +/* Alpha support old sysvipc even being a 64-bit architecture.  */
> +#undef __ASSUME_SYSVIPC_DEFAULT_IPC_64

s/support/requires/?

> diff --git a/sysdeps/unix/sysv/linux/alpha/ipc_priv.h b/sysdeps/unix/sysv/linux/alpha/ipc_priv.h
> index 881d943063..0bf9da5694 100644
> --- a/sysdeps/unix/sysv/linux/alpha/ipc_priv.h
> +++ b/sysdeps/unix/sysv/linux/alpha/ipc_priv.h

> +#define __OLD_IPC_SEQ_TYPE  unsigned short int
> +#include <sysdeps/unix/sysv/linux/ipc_priv.h>

Isn't that the __OLD_IPC_SEQ_TYPE default?  (This applies to other
architectures as well.

In fact, __OLD_IPC_SEQ_TYPE appears to be unsighed short int always, so
I think it shouldn't be parameterized.

> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h b/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h
> index 10d5c52dd0..459ee3d2e7 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/ipc_priv.h

> +#define __OLD_IPC_SEQ_TYPE   unsigned short int
> +#include <sysdeps/unix/sysv/linux/ipc_priv.h>

See above.

Rest looks okay to me.

Thanks,
Florian