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

[Laurent Vivier]

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:

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