Re: [PATCH v3 1/4] HID: wiimote: turn on the LEDs indicating the controller id
"Rafael Passos" <[email protected]> Sat, 01 Aug 2026 21:21:32 -0300
| Newsgroups | org.kernel.vger.linux-input |
|---|---|
| Message-ID | <[email protected]> |
On Fri Jul 31, 2026 at 6:02 AM -03, David Rheinsberg wrote:
>> - hid_info(hdev, "New device registered\n");
>> + player_id = ida_alloc_min(&wiimote_ida, 1, GFP_KERNEL);
>> + if (player_id < 1) {
>> + hid_err(hdev, "cannot allocate controller id\n");
>> + ret = player_id;
>> + goto err_free;
>> + }
>> +
>> + wdata->player_id = player_id;
>> +
>> + hid_info(hdev, "New device registered (Wiimote %d)\n", player_id);
>
> `wiimote_create()` should initialize `wdata->player_id` to -1, so `ida_free()` is a no-op in the `err_free` path. Right now it is implicitly initialized to 0, and thus ida_free() will warn about an unallocated ID.
>
> Also: Why not just use IDs starting from 0?
I chose to start from 1 to avoid "id 0 = player 1".
Wouldn't setting an u8 to -1 result into 255 ? If so,
this would not be a no-op. Also, comparing player_id < 0 would never be
true. If the IDA starts from 0, there is no usable "empty value" in a u8
when deciding if I should call ida_free.
If I let "failure" to be 0, I can just use this in destroy:
if (wdata->player_id)
ida_free(&wiimote_ida, wdata->player_id);
Thanks,
Rafael