Re: [PATCH] hid: fix incorrect return value for hid.

Re: [PATCH] hid: fix incorrect return value for hid.

[Michael Tokarev]

Please always send all patches to qemu-devel@ (cc'd)
(in this case, in addition to qemu-trivial@).

On 15.09.2025 20:57, ShengYi Hung wrote:
> The return value of hid_keyboard_write is used to set the packet's actual_length
> and pass to xhci directly to allow guest know how many byte actually processed.
> Therefore, return 1 to indicate a successful transfer or it will be
> considered as a wrong xfer.
> 
> Signed-off-by: ShengYi Hung <aokblast@FreeBSD.org>
> ---
>   hw/input/hid.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/input/hid.c b/hw/input/hid.c
> index 76bedc1844..de24cd0ef0 100644
> --- a/hw/input/hid.c
> +++ b/hw/input/hid.c
> @@ -478,6 +478,7 @@ int hid_keyboard_write(HIDState *hs, uint8_t *buf, int len)
>               ledstate |= QEMU_CAPS_LOCK_LED;
>           }
>           kbd_put_ledstate(ledstate);
> +        return 1;
>       }
>       return 0;
>   }

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

and queued up to the trivial-patches tree.

It's an interesting one, I wonder what the consequences are -
for having this at 0 all the time, and for actually making it 1.
Did we miss a byte somewhere with current code?

Thanks,

/mjt

Re: [PATCH] hid: fix incorrect return value for hid.

[ShengYi Hung]

Hello Michael:

Thanks for your review and reply, I sent the mail to qemud-devel about a
month ago but didn't receive any reply. Therefore, I send the mail to
qemu-trivial again and forget CC to qemu-devel. I will be careful next
time.

The consequence is that if the guest operating system want to check the
actual transfer length of a transfer, it may be regarded as a failed
transfer since the actual transfer length is zero originally.

Here is an example from FreeBSD:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288968.
I have workarounded this issue by ignore any error from hid_write.

Interestingly, Parallel seems to share the same issue and can be
addressed with the same fix.

Michael Tokarev <mjt@tls.msk.ru> writes:

> Please always send all patches to qemu-devel@ (cc'd)
> (in this case, in addition to qemu-trivial@).
>
> On 15.09.2025 20:57, ShengYi Hung wrote:
>> The return value of hid_keyboard_write is used to set the packet's actual_length
>> and pass to xhci directly to allow guest know how many byte actually processed.
>> Therefore, return 1 to indicate a successful transfer or it will be
>> considered as a wrong xfer.
>> Signed-off-by: ShengYi Hung <aokblast@FreeBSD.org>
>> ---
>>   hw/input/hid.c | 1 +
>>   1 file changed, 1 insertion(+)
>> diff --git a/hw/input/hid.c b/hw/input/hid.c
>> index 76bedc1844..de24cd0ef0 100644
>> --- a/hw/input/hid.c
>> +++ b/hw/input/hid.c
>> @@ -478,6 +478,7 @@ int hid_keyboard_write(HIDState *hs, uint8_t *buf, int len)
>>               ledstate |= QEMU_CAPS_LOCK_LED;
>>           }
>>           kbd_put_ledstate(ledstate);
>> +        return 1;
>>       }
>>       return 0;
>>   }
>
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
>
> and queued up to the trivial-patches tree.
>
> It's an interesting one, I wonder what the consequences are -
> for having this at 0 all the time, and for actually making it 1.
> Did we miss a byte somewhere with current code?
>
> Thanks,
>
> /mjt

-- 
Best Regards.
ShengYi Hung.

[PATCH] hid: fix incorrect return value for hid.

[ShengYi Hung]

The return value of hid_keyboard_write is used to set the packet's actual_length.
Therefore, return 1 to indicate a successful transfer or it should be
considered as a wrong xfer.

Signed-off-by: ShengYi Hung <aokblast@FreeBSD.org>
Sponsored by:   The FreeBSD Foundation
---
 hw/input/hid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/input/hid.c b/hw/input/hid.c
index 76bedc1844..5da9fee763 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -478,6 +478,7 @@ int hid_keyboard_write(HIDState *hs, uint8_t *buf, int len)
             ledstate |= QEMU_CAPS_LOCK_LED;
         }
         kbd_put_ledstate(ledstate);
+        return (1);
     }
     return 0;
 }
-- 
2.50.1