Re: [PATCH 02/17] Refactor Linux ipc_priv header

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 12/12/2016 10:47, Arnd Bergmann wrote:
> On Monday, December 12, 2016 10:32:55 AM CET Adhemerval Zanella wrote:
> 
>> Since current IPC_64 default now on kernel is 0x100, some architectures
>> require it to 0 (for instance x86_64) while others continue to use a
>> non zero default regardless (powerpc).
> 
> I find the explanation is a bit confusing, it's not that the kernel
> requires IPC_64 to be 0x100, it's that some architectures support
> the the old-style IPC and require IPC_64 to be passed, while new
> architectures should default to the new version.

Right, I did not want to expose some kernel internal implementation,
but I think your explanation should be better.  I will change to

"
Some architectures support the old-style IPC and require IPC_64
equal to 0x100 to be passed along SysV IPC syscalls, while new
architectures should default to new IPC version (without the
flags being set).

This patch refactor current ipc_priv.h Linux headers in two directions:

  - Remove cross platform references (for instance alpha including powerpc
    definition) and add required definition for each required port.  The
    idea is to avoid tie one architecture definition with another and make
    platform change independent.

  - Move all common definitions (the ipc syscall commands) on a common
    header, ipc_ops.h.
"

Do you think it is less confusing?

> 
>> index 0000000..1a31396
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/aarch64/ipc_priv.h
> ...
>> +
>> +#include <sys/ipc.h>  /* For __key_t  */
>> +
>> +#define __IPC_64	0x0
>> +
>> +struct __old_ipc_perm
>> +{
>> +  __key_t __key;		/* Key.  */
>> +  unsigned int uid;		/* Owner's user ID.  */
>> +  unsigned int gid;		/* Owner's group ID.  */
>> +  unsigned int cuid;		/* Creator's user ID.  */
>> +  unsigned int cgid;		/* Creator's group ID.  */
>> +  unsigned int mode;		/* Read/write permission.  */
>> +  unsigned short int __seq;	/* Sequence number.  */
>> +};
> 
> I don't understand this part at all: the #define line first says
> that the old IPC is not supported, but then you define the structure
> anyway?
> 
> What is the structure actually used for in glibc? My interpretation
> is that it's only needed to implement the SHLIB_COMPAT version
> of the library calls that don't exist on ARM64 either.

You are right and it is mainly a limitation on how my code defines the
'union semun' in semctl.c.  I will change it to use an opaque type
instead of a pointer to __old_ipc_perm.