Re: [Qemu-devel] [PATCH 07/10] linux-user: init_guest_space: Clean up control flow a bit

[Peter Maydell]

On 13 March 2018 at 13:30, Laurent Vivier <address@hidden> wrote:
> Le 09/03/2018 à 21:37, Laurent Vivier a écrit :
>> Le 28/12/2017 à 19:08, Luke Shumaker a écrit :
>>> --- a/linux-user/elfload.c
>>> +++ b/linux-user/elfload.c
>>> @@ -1906,24 +1906,28 @@ unsigned long init_guest_space(unsigned long 
>>> host_start,
>>>          }
>>>
>>>          /* Check to see if the address is valid.  */
>>> -        if (!host_start || aligned_start == current_start) {
>>> +        if (host_start && aligned_start != current_start) {
>>> +            goto try_again;
>>> +        }
>>> +
>>>  #if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
>>> -            /* On 32-bit ARM, we need to also be able to map the commpage. 
>>>  */
>>> -            int valid = init_guest_commpage(aligned_start - guest_start,
>>> -                                            aligned_size + guest_start);
>>> -            if (valid == 1) {
>>> -                break;
>>> -            } else if (valid == -1) {
>>> -                munmap((void *)real_start, real_size);
>>> -                return (unsigned long)-1;
>>> -            }
>>> -            /* valid == 0, so try again. */
>>> -#else
>>> -            /* On other architectures, whatever we have here is fine.  */
>>> -            break;
>>> -#endif
>>> +        /* On 32-bit ARM, we need to also be able to map the commpage.  */
>>> +        int valid = init_guest_commpage(aligned_start - guest_start,
>>> +                                        aligned_size + guest_start);
>>> +        if (valid == -1) {
>>> +            munmap((void *)real_start, real_size);
>>> +            return (unsigned long)-1;
>>> +        } else if (valid == -1) {
>>
>> I think it should be "if (valid == 0)" here.
>
> Any comment?

Yes, I think I agree.

thanks
-- PMM