Re: [Qemu-devel] [PATCH v1 05/23] semihosting: enable chardev backed output

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 5/9/19 9:58 AM, Alex Bennée wrote:
>> @@ -51,12 +51,18 @@ static inline const char *semihosting_get_cmdline(void)
>>  {
>>      return NULL;
>>  }
>> +
>> +static inline Chardev *semihosting_get_chardev(void)
>> +{
>> +    return NULL;
>> +}
>
> Isn't the point of this function to avoid...

Yes... but...

>
>> -                return write(STDERR_FILENO, &c, 1);
>> +#ifdef CONFIG_SOFTMMU
>> +              Chardev *chardev = semihosting_get_chardev();
>> +              if (chardev) {
>> +                  return qemu_chr_write_all(chardev, (uint8_t *) &c,
>1);

The qemu_chr device stuff doesn't have such stubs and is softmmu only as
well. *sigh*

I guess stub it out in the header as well?

>> +              } else
>> +#endif
>> +              {
>> +                  return write(STDERR_FILENO, &c, 1);
>> +              }
>
> ... this ifdef?
>
> Better to change
>
> - char c;
> + uint8_t c;
>
> above to avoid the cast in the call to qemu_chr_write_all?
> Or perhaps we should adjust qemu_chr_write_all to take void*?
>
>
> r~


--
Alex Bennée