From: Alex Elder <elder@linaro.org>
To: Jakob Koschel <jakobkoschel@gmail.com>,
Dan Carpenter <dan.carpenter@oracle.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev,
Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
Mike Rapport <rppt@kernel.org>,
Brian Johannesmeyer <bjohannesmeyer@gmail.com>,
Cristiano Giuffrida <c.giuffrida@vu.nl>,
"Bos, H.J." <h.j.bos@vu.nl>
Subject: Re: [greybus-dev] Re: [PATCH] staging: greybus: codecs: fix type confusion with dedicated list iterator variable
Date: Mon, 21 Mar 2022 07:18:57 -0500 [thread overview]
Message-ID: <312061ed-61d6-4e2f-a180-8e8c85573e31@linaro.org> (raw)
In-Reply-To: <56102C8D-DBFF-43A5-9641-04577F01AB6D@gmail.com>
On 3/21/22 4:27 AM, Jakob Koschel wrote:
>
I just released some messages that were marked as spam. So
this looks to me like it's already been seen on the list.
I'm not sure why this happens but if it seems like deja vu,
you're not imagining things. Please know that this could
happen from time to time, but I'll see if I can find out
how to avoid it.
-Alex
>> On 21. Mar 2022, at 10:21, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>
>> On Mon, Mar 21, 2022 at 10:06:13AM +0100, Jakob Koschel wrote:
>>>
>>>> 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.
>>
>> Ah. I did not read all the way to the end of the patch.
>>
>> The bugfix needs to be sent as it's own patch. Just the one liner. It
>> needs a fixes tag as well.
>>
>> [PATCH] staging: greybus: fix Oops in error message
>>
>> The "module" pointer is invalid here. It's the list iterator and we
>> exited the loop without finding a valid entry.
>>
>> Fixes: 6dd67645f22c ("greybus: audio: Use single codec driver registration")
>> Signed-off-by: You
>>
>> if (!data) {
>> - dev_err(dai->dev, "%s:%s DATA connection missing\n",
>> - dai->name, module->name);
>> + dev_err(dai->dev, "%s DATA connection missing\n",
>> + dai->name);
>> mutex_unlock(&codec->lock);
>>
>> We're happy to apply the other stuff as well, but we don't mix cleanups
>> and bug fixes like that.
>
> ok great, I'll separate and resubmit both. Thanks!
>
>>
>> regards,
>> dan carpenter
>
> Jakob
>
> _______________________________________________
> greybus-dev mailing list -- greybus-dev@lists.linaro.org
> To unsubscribe send an email to greybus-dev-leave@lists.linaro.org
prev parent reply other threads:[~2022-03-21 12:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-19 20:20 [PATCH] staging: greybus: codecs: fix type confusion with dedicated list iterator variable Jakob Koschel
2022-03-21 8:48 ` Dan Carpenter
2022-03-21 9:06 ` Jakob Koschel
2022-03-21 9:21 ` Dan Carpenter
2022-03-21 9:27 ` Jakob Koschel
2022-03-21 12:18 ` Alex Elder [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=312061ed-61d6-4e2f-a180-8e8c85573e31@linaro.org \
--to=elder@linaro.org \
--cc=bjohannesmeyer@gmail.com \
--cc=c.giuffrida@vu.nl \
--cc=dan.carpenter@oracle.com \
--cc=elder@kernel.org \
--cc=greybus-dev@lists.linaro.org \
--cc=h.j.bos@vu.nl \
--cc=jakobkoschel@gmail.com \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=rppt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox