Re: [Qemu-trivial] [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 à 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.

Laurent