Laurent Vivier <laur...@vivier.eu> 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 <alex.ben...@linaro.org>
>> ---
<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
