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

["Manszewski, Christoph" <christoph.manszewski@intel.com>]

Hi Peter,

On 23.09.2025 11:20, Peter Senna Tschudin wrote:
> 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

Well either way is fine with by me. I just wanted to make a point that 
when we add a check that catches a bug it makes sense to alarm the 
developer right away.

Regards,
Christoph

> 
> Nacked-by: Peter Senna Tschudin <peter.senna@linux.intel.com>
> 
>>
>> Regards,
>> Christoph
>>
>>
>>>        return head->next == head;
>>>    }
>>
>