Re: [Qemu-devel] [PATCH for 2.10 26/35] linux-user: use is_error() to avoid warnings and make the code clearer

[Philippe Mathieu-Daudé]

On 06/03/2018 08:33 PM, Laurent Vivier wrote:
> Le 29/05/2018 à 17:19, Laurent Vivier a écrit :
>> Le 29/05/2018 à 16:25, Philippe Mathieu-Daudé a écrit :
>>> Hi Laurent,
>>
>> Hi Philippe,
>>
>>> On 07/24/2017 04:16 PM, Laurent Vivier wrote:
>>>> Le 24/07/2017 à 20:27, Philippe Mathieu-Daudé a écrit :
>>>>> linux-user/flatload.c:740:9: warning: Loss of sign in implicit conversion
>>>>>     if (res > (unsigned long)-4096)
>>>>>         ^~~
>>>>>
>>>>> Reported-by: Clang Static Analyzer
>>>>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>>>>
>>>> Reviewed-by: Laurent Vivier <address@hidden>
>>>>
>>>>> ---
>>>>>  linux-user/flatload.c | 15 +++++++++------
>>>>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
>>>>> index a35a560904..10c529910f 100644
>>>>> --- a/linux-user/flatload.c
>>>>> +++ b/linux-user/flatload.c
>>>>> @@ -224,8 +224,9 @@ static int decompress_exec(
>>>>>           ret = bprm->file->f_op->read(bprm->file, buf, LBUFSIZE, &fpos);
>>>>>           if (ret <= 0)
>>>>>                   break;
>>>>> -         if (ret >= (unsigned long) -4096)
>>>>> +                if (is_error(ret)) {
>>>>>                   break;
>>>>> +                }
>>>>>           len -= ret;
>>>>>  
>>>>>           strm.next_in = buf;
>>>>> @@ -283,8 +284,7 @@ calc_reloc(abi_ulong r, struct lib_info *p, int 
>>>>> curid, int internalp)
>>>>>                      "in same module (%d != %d)\n",
>>>>>                      (unsigned) r, curid, id);
>>>>>              goto failed;
>>>>> -        } else if ( ! p[id].loaded &&
>>>>> -                    load_flat_shared_library(id, p) > (unsigned long) 
>>>>> -4096) {
>>>>> +        } else if (!p[id].loaded && 
>>>>> is_error(load_flat_shared_library(id, p))) {
>>>>>              fprintf(stderr, "BINFMT_FLAT: failed to load library %d\n", 
>>>>> id);
>>>>>              goto failed;
>>>>>          }
>>>>> @@ -523,9 +523,10 @@ static int load_flat_file(struct linux_binprm * bprm,
>>>>>                  fpos = 0;
>>>>>                  result = bprm->file->f_op->read(bprm->file,
>>>>>                                  (char *) textpos, text_len, &fpos);
>>>>> -                if (result < (unsigned long) -4096)
>>>>> +                if (!is_error(result)) {
>>>>>                          result = decompress_exec(bprm, text_len, (char 
>>>>> *) datapos,
>>>>>                                           data_len + (relocs * 
>>>>> sizeof(unsigned long)), 0);
>>>>> +                }
>>>>>          }
>>>>>          else
>>>>>  #endif
>>>>> @@ -693,8 +694,9 @@ static int load_flat_shared_library(int id, struct 
>>>>> lib_info *libs)
>>>>>  
>>>>>   res = prepare_binprm(&bprm);
>>>>>  
>>>>> - if (res <= (unsigned long)-4096)
>>>>> +        if (!is_error(res)) {
>>>>>           res = load_flat_file(&bprm, libs, id, NULL);
>>>>> +        }
>>>>>   if (bprm.file) {
>>>>>           allow_write_access(bprm.file);
>>>>>           fput(bprm.file);
>>>>> @@ -737,8 +739,9 @@ int load_flt_binary(struct linux_binprm *bprm, struct 
>>>>> image_info *info)
>>>>>  
>>>>>  
>>>>>      res = load_flat_file(bprm, libinfo, 0, &stack_len);
>>>>> -    if (res > (unsigned long)-4096)
>>>>> +    if (is_error(res)) {
>>>>>              return res;
>>>>> +    }
>>>>>  
>>>>>      /* Update data segment pointers for all libraries */
>>>>>      for (i=0; i<MAX_SHARED_LIBS; i++) {
>>>>>
>>>
>>> Can you take this via your linux-user tree?
>>>
>>
>> Applied, thanks.
> 
> Unapplied, it needs a rebase:

No rebase required, it just need the previous patch applied too :)
But I didn't think of explicit it :/
Can you take both of them directly or do you prefer I RESEND?

> 
> qemu/linux-user/flatload.c: In function 'load_flt_binary':
> qemu/linux-user/flatload.c:742:9: error: implicit declaration of
> function 'is_error'; did you mean 'g_error'?
> [-Werror=implicit-function-declaration]
>      if (is_error(res)) {
>          ^~~~~~~~
>          g_error
> qemu/linux-user/flatload.c:742:9: error: nested extern declaration of
> 'is_error' [-Werror=nested-externs]
> 
> Thanks,
> Laurent
> 
> 
>