> On 21. Mar 2022, at 09:48, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> The subject says that it fixes a bug but it does not.
Thank you for your review!
I don't agree that this doesn't fix a bug:
> + }
> }
> if (!data) {
> - dev_err(dai->dev, "%s:%s DATA connection missing\n",
> - dai->name, module->name);
Using 'module' when data == NULL is *guaranteed* to be a type confused
bogus pointer. It fundamentally can never be correct.
If I should still change the wording please let me know.
> + dev_err(dai->dev, "%s DATA connection missing\n",
> + dai->name);
> mutex_unlock(&codec->lock);
> return -ENODEV;
> }
>
> On Sat, Mar 19, 2022 at 09:20:58PM +0100, Jakob Koschel wrote:
>> If the list does not exit early then data == NULL and 'module' does not
>> point to a valid list element.
>> Using 'module' in such a case is not valid and was therefore removed.
>
> This paragraph is confusing jumble words. Just say: "This code is fine".
>
>>
>> In preparation to limit the scope of the list iterator to the list
>> traversal loop, use a dedicated pointer pointing to the found element [1].
>
> This paragraph is the information we need. Just add something like
> "This patch has no effect on runtime".
As mentioned above, this code effects runtime (in one out of the two cases).
>
> regards,
> dan carpenter
Thanks,
Jakob
_______________________________________________
greybus-dev mailing list -- greybus-dev@xxxxxxxxxxxxxxxx
To unsubscribe send an email to greybus-dev-leave@xxxxxxxxxxxxxxxx
|