Re: [Qemu-devel] [PATCH v4] linux-user: remove useless padding in flock64 structure

[Richard Henderson <richard.henderson@linaro.org>]

On 05/02/2018 02:57 PM, Laurent Vivier wrote:
> Since commit 8efb2ed5ec ("linux-user: Correct signedness of
> target_flock l_start and l_len fields"), flock64 structure uses
> abi_llong for l_start and l_len in place of "unsigned long long"
> this should force them to be aligned accordingly to the target
> rules. So we can remove the padding field and the QEMU_PACKED
> attribute.
> 
> I have compared the result of the following program before and
> after the change:
> 
>     cat -> flock64_dump  <<EOF
>     p/d sizeof(struct target_flock64)
>     p/d &((struct target_flock64 *)0)->l_type
>     p/d &((struct target_flock64 *)0)->l_whence
>     p/d &((struct target_flock64 *)0)->l_start
>     p/d &((struct target_flock64 *)0)->l_len
>     p/d &((struct target_flock64 *)0)->l_pid
>     quit
>     EOF
> 
>     for file in build/all/*-linux-user/qemu-* ; do
>     echo $file
>     gdb -batch -nx -x flock64_dump $file 2> /dev/null
>     done
> 
> The sizeof() changes because we remove the QEMU_PACKED.
> The new size is 32 (except for i386 and m68k) and this is
> the real size of "struct flock64" on the target architecture.
> 
> The following architectures differ:
> aarch64_be, aarch64, alpha, armeb, arm, cris, hppa, nios2, or1k,
> riscv32, riscv64, s390x.
> 
> For a subset of these architectures, I have checked with the following
> program the new structure is the correct one:
> 
>   #include <stdio.h>
>   #define __USE_LARGEFILE64
>   #include <fcntl.h>
> 
>   int main(void)
>   {
> 	  printf("struct flock64 %d\n", sizeof(struct flock64));
> 	  printf("l_type %d\n", &((struct flock64 *)0)->l_type);
> 	  printf("l_whence %d\n", &((struct flock64 *)0)->l_whence);
> 	  printf("l_start %d\n", &((struct flock64 *)0)->l_start);
> 	  printf("l_len %d\n", &((struct flock64 *)0)->l_len);
> 	  printf("l_pid %d\n", &((struct flock64 *)0)->l_pid);
>   }
> 
> [I have checked aarch64, alpha, hppa, s390x]
> 
> For ARM, the target_flock64 becomes the EABI definition, so we need to
> define the OABI one in place of the EABI one and use it when it is
> needed.
> 
> I have also fixed the alignment value for sh4 (to align llong on 4 bytes)
> (see c2e3dee6e0 "linux-user: Define target alignment size")
> [We should check alignment properties for cris, nios2 and or1k]

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~