lkml.org 
[lkml]   [2022]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] staging: greybus: codecs: fix type confusion with dedicated list iterator variable
From
Date

> On 21. Mar 2022, at 09:48, Dan Carpenter <dan.carpenter@oracle.com> 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

\
 
 \ /
  Last update: 2022-03-21 10:07    [W:1.424 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site