Re: Remove __ASSUME_ATFCTS conditionals in sysdeps/unix/sysv/linux/

[Aurelien Jarno <aurelien at aurel32 dot net>]

On Fri, May 16, 2014 at 12:19:53AM +0000, Joseph S. Myers wrote:
> This patch cleans up for __ASSUME_ATFCTS now always being true for the
> supported Linux kernel versions by removing conditional code in
> sysdeps/unix/sysv/linux.  Several fchownat.c files that were only
> present because of differences in the fallback syscalls used
> (depending on the architecture-specific names of chown-related
> syscalls for 32-bit uids) are removed.  Files that looks like they
> could be replaced by syscalls.list entries have the standard "Consider
> moving to syscalls.list." comment (see bug 14138) added.  Conditionals
> on the relevant __NR_* syscall numbers being defined are also removed,
> since my analysis indicated that the relevant syscalls are always
> defined for all relevant kernel versions using any affected file.
> Much of the removed fallback code had unbounded stack allocations, so
> this reduces the number of cases to consider for anyone reviewing uses
> of alloca and VLAs in glibc.
> 
> There remain tests of __ASSUME_ATFCTS in io/openat.c (to determine
> whether to define __have_atfcts) and sysdeps/posix/getcwd.c (which
> also uses __have_atfcts); thus, the definition of __ASSUME_ATFCTS
> remains in kernel-features.h.  The logical condition relevant there is
> whether openat64_not_cancel_3 is known to work.  Hurd doesn't use this
> version of getcwd at all, so the conditionals in getcwd.c are always
> true in glibc.  However, this code is also used in gnulib.  So the
> best way to deal with the conditionals there may be for gnulib people
> to deal with merging all relevant changes in both directions between
> the glibc and gnulib versions of this file, at the end of which the
> openat conditionals should be in whatever form is best for gnulib, and
> hardcoded in the _LIBC case to having openat supported.
> 
> Tested by comparing before-and-after disassembly of installed
> (stripped) shared libraries, on x86_64 and x86.  On x86 the patch made
> no change to the disassembly; on x86_64, the only changes were in
> readlinkat, where formerly the return value from the readlinkat
> syscall was stored in an int variable before being converted to
> ssize_t for the return, and now the return value is returned directly
> without truncation to int.  I think it's clearly correct not to
> truncate the return value (although I also think the truncation would
> not have been a user-visible bug because the kernel would never have
> returned a value it could have affected).

This looks fine to me. After this cleanup, I think the various 
dl-fxstatat64.c copies can be removed (though I haven't tested that),
but I guess you prefer to handle that in a different patch set.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net