Am 14.07.20 um 10:25 schrieb Dan Carpenter:
> On Tue, Jul 14, 2020 at 03:39:13AM +0200, Roland Scheidegger wrote:
>> Am 26.06.20 um 12:39 schrieb Dan Carpenter:
>>> These if statements are supposed to be true if we ended the
>>> list_for_each_entry() loops without hitting a break statement but they
>>> don't work.
>>>
>>> In the first loop, we increment "i" after the "if (i == unit)" condition
>>> so we don't necessarily know that "i" is not equal to unit at the end of
>>> the loop.
>> So, if I understand this right, this would only really be a problem if
>> there's no list entries at all, right? That is i == unit == 0.
>> Not sure if that can actually happen, but in any case the fix looks correct.
>
> An empty list and there is another potential issue where unit is exactly
> off by one.
>
> list_for_each_entry(con, &dev_priv->dev->mode_config.connector_list,
> head) {
> if (i == unit)
> break;
> ++i; <-- this is the last iteration and it's off by one
> so now i == unit but we didn't exit via the
> break statement.
> }
>
> if (i != unit) {
> ^^^^^^^^^
> Since we didn't exit by the break statement we want this to be true but
> it's false instead.
>
> DRM_ERROR("Could not find initial display unit.\n");
>
> I don't know how *likely* this is, but static checkers complain.
> Technically correct is the best kind of correct! ;)
Ahh indeed seems obvious now. But kinda difficult to spot :-).
Thanks again,
Roland
>
> regards,
> dan carpenter
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
