Re: [PATCH i-g-t] lib/igt_list: igt_list_empty() Detect uninitialized list

[Peter Senna Tschudin <peter.senna@linux.intel.com>]

Hi Christoph,

On 9/23/2025 10:14 AM, Manszewski, Christoph wrote:
> Hi Peter,
> 
> On 22.09.2025 18:23, Peter Senna Tschudin wrote:
>> Add a check to igt_list_empty() to return true if the list head pointer
>> or its next/prev pointers are NULL, preventing segfaults when operating
>> on uninitialized or potentially broken lists.
>>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
>> ---
>>   lib/igt_list.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/igt_list.c b/lib/igt_list.c
>> index 37ae139c4..f77aef7dd 100644
>> --- a/lib/igt_list.c
>> +++ b/lib/igt_list.c
>> @@ -79,5 +79,9 @@ int igt_list_length(const struct igt_list_head *head)
>>     bool igt_list_empty(const struct igt_list_head *head)
>>   {
>> +    /* Detect uninitialized / potentially broken list */
>> +    if (!head || !head->next || !head->prev)
>> +        return true;
>> +
> 
> Without this change:
> 1. Passing an uninitialized list is a bug
> 2. The function doesn't check any condition and returns 'normally'
> regardless
> 
> With this change:
> 1. Passing an uninitialized list is a bug (still)
> 2. The function *may* detect an uninitialized list which is a bug but
> returns 'normally' regardless
> 
> If we are adding a check that catches some invalid/buggy condition the
> reasonable thing to do would be to assert/abort.

I agree with you. I would say that even more concerning is to distance
our implementation from the kernel implementation. I suggest we nack
this change. If you are ok with it

Nacked-by: Peter Senna Tschudin <peter.senna@linux.intel.com>

> 
> Regards,
> Christoph
> 
> 
>>       return head->next == head;
>>   }
>