Re: [Qemu-arm] [RFC PATCH] semihosting: split console_out intro string and char versions

[Alex Bennée]

Laurent Vivier <address@hidden> writes:

> Le 30/05/2019 à 16:39, Alex Bennée a écrit :
>> This is ostensibly to avoid the weirdness of len looking like it might
>> come from a guest and sometimes being used. While we are at it fix up
>> the error checking for the arm-linux-user implementation of the API
>> which got flagged up by Coverity (CID 1401700).
>>
>> Signed-off-by: Alex Bennée <address@hidden>
>> ---
<snip>
>>  /**
>>   * qemu_semihosting_log_out:
>> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
>> index 9554102a855..b7cdc21f832 100644
>> --- a/linux-user/arm/semihost.c
>> +++ b/linux-user/arm/semihost.c
>> @@ -15,10 +15,34 @@
>>  #include "hw/semihosting/console.h"
>>  #include "qemu.h"
>>
>> -int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int 
>> len)
>> +int qemu_semihosting_console_outs(CPUArchState *env, target_ulong addr)
>>  {
>> +    int len;
>>      void *s = lock_user_string(addr);
>> -    len = write(STDERR_FILENO, s, len ? len : strlen(s));
>> +    if (!s) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: passed inaccessible address " TARGET_FMT_lx,
>> +                      __func__, addr);
>> +        return 0;
>> +    }
>> +
>> +    len = write(STDERR_FILENO, s, strlen(s));
>
> You could avoid 2 calls to strlen() if you inline directly
> lock_user_string() content:
>
>     len = target_strlen(addr);
>     if (len < 0){
>        qemu_log_mask(LOG_GUEST_ERROR,
>                      "%s: passed inaccessible address " TARGET_FMT_lx,
>                      __func__, addr);
>        return 0;
>     }
>     s = lock_user(VERIFY_READ, addr, (long)(len + 1), 1);
>     len = write(STDERR_FILENO, s, len);
>
>>      unlock_user(s, addr, 0);
>>      return len;
>>  }

It's a nice clean-up but I've just looked at what was going on inside
with lock_user. I guess g_assert(s) on the lock user would be fair as we
can't fail at this point?

--
Alex Bennée