Re: [Qemu-devel] [PATCH v2] linux-user: Use *at functions instead of caching interp_prefix contents

[Richard Henderson]

On 12/07/2017 05:05 AM, Peter Maydell wrote:
> On 5 December 2017 at 04:48, Richard Henderson
> <address@hidden> wrote:
>> From: Richard Henderson <address@hidden>
>>
>> If the interp_prefix is a complete chroot, it may have a *lot* of files.
>> Setting up the cache for this is quite expensive.  Instead, use the *at
>> versions of various syscalls to attempt the operation in the prefix.
>>
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>
>> Changes since v1 (Nov 2016):
>>   * Require interp_dirfd set before trying the *at path.
>>
>>
>> r~
>> ---
>>  linux-user/qemu.h    |   1 +
>>  linux-user/elfload.c |  12 ++-
>>  linux-user/main.c    |   3 +-
>>  linux-user/syscall.c | 236 
>> ++++++++++++++++++++++++++++++++++++++++++---------
>>  4 files changed, 208 insertions(+), 44 deletions(-)
>>
>> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
>> index 4edd7d0c08..d04924fd2e 100644
>> --- a/linux-user/qemu.h
>> +++ b/linux-user/qemu.h
>> @@ -437,6 +437,7 @@ void mmap_fork_start(void);
>>  void mmap_fork_end(int child);
>>
>>  /* main.c */
>> +extern int interp_dirfd;
>>  extern unsigned long guest_stack_size;
>>
>>  /* user access */
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 20f3d8c2c3..5d3f7aa11d 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2203,7 +2203,17 @@ static void load_elf_interp(const char *filename, 
>> struct image_info *info,
>>  {
>>      int fd, retval;
>>
>> -    fd = open(path(filename), O_RDONLY);
>> +    switch (interp_dirfd > 0 && filename[0] == '/') {
>> +    case true:
> 
> gcc doesn't like this pattern:
> 
> /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c: In
> function ‘do_name_to_handle_at’:
> /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c:7231:13:
> error: switch condition has boolean value [-Werror=switch-bool]
>      switch (interp_dirfd > 0 && name[0] == '/') {
>              ^
> 
> It seems like an odd way to write an if().

Huh.  My gcc likes it just fine.  What version are you using?

It's an IF with a /* fallthru */, so there are very few
less ugly ways to write it...


r~