Re: [PATCH 3/4] Consolidate Linux recv and recvfrom implementation

["Dmitry V. Levin" <ldv at altlinux dot org>]

On Mon, Jan 30, 2017 at 12:02:05PM -0200, Adhemerval Zanella wrote:
> This patch consolidates the recv Linux syscall implementation on
> sysdeps/unix/sysv/linux/recv.c.  The changes are:
> 
>   1. Remove recv and recvfrom from auto-generation syscalls.list on the
>      architecture that uses __NR_recv.

Please split this for correctness, e.g.

  # Remove recvfrom from auto-generation syscalls.list on all Linux
    architectures.
  # Remove recv from auto-generation syscalls.list on architectures that
    use __NR_recv.

>   2. Define __NR_recv for such architectures.

You surely don't define __NR_* for any architecture, do you?

>      It was done instead of
>      defining in default kernel-features.h because current Linux practice
>      for new ports are to implement only __NR_recvfrom [1] and it will
>      require adding new kernel-features for ports that do not require it
>      (aarch64 for instance).
>   3. Define __NR_recvfrom as default (__ASSUME_RECVFROM_SYSCALL) and undef
>      for architectures that do not support it.

This is also misleading, you probably mean

  # Define __ASSUME_RECVFROM_SYSCALL by default, undef it for
    architectures that do not support __NR_recvfrom.

>   4. Remove __ASSUME_RECVFROM_FOR_RECV_SYSCALL and decide to use
>      __NR_recvfrom for recv generation based on __ASSUME_RECVFROM__SYSCALL.

Too many underscores in "__ASSUME_RECVFROM__SYSCALL".

> Checked on i686-linux-gnu, x86_64-linux-gnu, x86_64-linux-gnux32,
> aarch64-linux-gnu, arm-linux-gnueabihf, and powerpc64le-linux-gnu.
> 
> 	* sysdeps/unix/sysv/linux/alpha/kernel-features.h
> 	(__ASSUME_RECV_SYSCALL): Define.
> 	* sysdeps/unix/sysv/linux/arm/kernel-features.h
> 	(__ASSUME_RECV_SYSCALL): Likewise.
> 	* sysdeps/unix/sysv/linux/hppa/kernel-features.h
> 	(__ASSUME_RECV_SYSCALL): Likewise.
> 	* sysdeps/unix/sysv/linux/ia64/kernel-features.h
> 	(__ASSUME_RECV_SYSCALL): Likewise.
> 	* sysdeps/unix/sysv/linux/mips/kernel-features.h
> 	[_MIPS_SIM == _ABIO32] (__ASSUME_RECV_SYSCALL): Likewise.
> 	* sysdeps/unix/sysv/linux/i386/kernel-features.h
> 	(__ASSUME_RECVFROM_SYSCALL): Define whether the kernel supports it.

This is misleading: the change undefs __ASSUME_RECVFROM_SYSCALL when
the kernel does not support __NR_recvfrom.

> 	(__ASSUME_RECVFROM_FOR_RECV_SYSCALL): Undefine.

The removal of #define is not undefine, it's remove.


-- 
ldv

Attachment: pgpmvUe6UW0ol.pgp
Description: PGP signature