Messages in this thread | | | Subject | Re: [PATCH] staging: greybus: codecs: fix type confusion with dedicated list iterator variable | From | Jakob Koschel <> | Date | Mon, 21 Mar 2022 10:06:13 +0100 |
| |
> 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
| |