Re: [PATCH v3 1/4] HID: wiimote: turn on the LEDs indicating the controller id

Hi

On Sun, Aug 2, 2026, at 2:21 AM, Rafael Passos wrote:
> 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".

I think this would be perfectly fine, but I don't insist on changing this.

> 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);

Whatever you decide on using as invalid value, you need to check it before calling into ida_free(). If you reserve 0, use what you suggested. If you put -1 in, check for -1 / 255 (even though technically you should then also limit the allocations to 254). Or change the type to s8 or int, or whatever works.

Thanks
David