Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1717476 > unrolled thread

[PATCH] ASoC: Add a sanity check before using dai driver name

Started byJeffy Chen <jeffy.chen@rock-chips.com>
First post2017-08-22 16:50 +0200
Last post2017-08-24 12:50 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] ASoC: Add a sanity check before using dai driver name Jeffy Chen <jeffy.chen@rock-chips.com> - 2017-08-22 16:50 +0200
    Re: [PATCH] ASoC: Add a sanity check before using dai driver name Mark Brown <broonie@kernel.org> - 2017-08-22 18:20 +0200
      Re: [PATCH] ASoC: Add a sanity check before using dai driver name jeffy <jeffy.chen@rock-chips.com> - 2017-08-23 01:50 +0200
        Re: [PATCH] ASoC: Add a sanity check before using dai driver name Mark Brown <broonie@kernel.org> - 2017-08-23 13:10 +0200
          Re: [PATCH] ASoC: Add a sanity check before using dai driver name jeffy <jeffy.chen@rock-chips.com> - 2017-08-24 05:30 +0200
            Re: [PATCH] ASoC: Add a sanity check before using dai driver name Mark Brown <broonie@kernel.org> - 2017-08-24 12:20 +0200
              Re: [PATCH] ASoC: Add a sanity check before using dai driver name jeffy <jeffy.chen@rock-chips.com> - 2017-08-24 12:50 +0200

#1717476 — [PATCH] ASoC: Add a sanity check before using dai driver name

FromJeffy Chen <jeffy.chen@rock-chips.com>
Date2017-08-22 16:50 +0200
Subject[PATCH] ASoC: Add a sanity check before using dai driver name
Message-ID<uhiFc-3Hr-21@gated-at.bofh.it>
The dai driver's name is allowed to be NULL. So add a sanity check for
that.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Reported-by: Donglin Peng <dolinux.peng@gmail.com>
---

 sound/soc/soc-core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 77e7e2a11af0..8ab9ee2b460a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -978,11 +978,13 @@ struct snd_soc_dai *snd_soc_find_dai(
 		if (dlc->name && strcmp(component->name, dlc->name))
 			continue;
 		list_for_each_entry(dai, &component->dai_list, list) {
-			if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
-			    && strcmp(dai->driver->name, dlc->dai_name))
-				continue;
-
-			return dai;
+			if (!dlc->dai_name)
+				return dai;
+			if (!strcmp(dai->name, dlc->dai_name))
+				return dai;
+			if (dai->driver->name &&
+			    !strcmp(dai->driver->name, dlc->dai_name))
+				return dai;
 		}
 	}
 
-- 
2.11.0

[toc] | [next] | [standalone]

#1717553

FromMark Brown <broonie@kernel.org>
Date2017-08-22 18:20 +0200
Message-ID<uhk4i-4NV-27@gated-at.bofh.it>
In reply to#1717476

[Multipart message — attachments visible in raw view] — view raw

On Tue, Aug 22, 2017 at 10:45:12PM +0800, Jeffy Chen wrote:

> -			return dai;
> +			if (!dlc->dai_name)
> +				return dai;
> +			if (!strcmp(dai->name, dlc->dai_name))
> +				return dai;

You want (dlc->dai_name && !strcmp(dai->name, dlc->dai_name)) for this
to be equivalent don't you?

> +			if (dai->driver->name &&
> +			    !strcmp(dai->driver->name, dlc->dai_name))

[toc] | [prev] | [next] | [standalone]

#1717897

Fromjeffy <jeffy.chen@rock-chips.com>
Date2017-08-23 01:50 +0200
Message-ID<uhr5L-1aI-5@gated-at.bofh.it>
In reply to#1717553
hi Mark,

On 08/23/2017 12:17 AM, Mark Brown wrote:
> On Tue, Aug 22, 2017 at 10:45:12PM +0800, Jeffy Chen wrote:
>
>> -			return dai;
>> +			if (!dlc->dai_name)
>> +				return dai;
>> +			if (!strcmp(dai->name, dlc->dai_name))
>> +				return dai;
>
> You want (dlc->dai_name && !strcmp(dai->name, dlc->dai_name)) for this
> to be equivalent don't you?
i think the original check is allowing NULL dlc dai_name to be a match...
so we basically did:
reject when dlc dai_name is valid, but not match the dai name

and my patch is:
accept when dlc dai_name is invalid
accept when match dai name
accept when match dai driver name(only when it is valid)

so it's a "if (a && b) reject" to "if (!a || !b) accept" case..

>
>> +			if (dai->driver->name &&
>> +			    !strcmp(dai->driver->name, dlc->dai_name))
>

[toc] | [prev] | [next] | [standalone]

#1718252

FromMark Brown <broonie@kernel.org>
Date2017-08-23 13:10 +0200
Message-ID<uhBHQ-8bC-23@gated-at.bofh.it>
In reply to#1717897

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 23, 2017 at 07:42:55AM +0800, jeffy wrote:

> i think the original check is allowing NULL dlc dai_name to be a match...
> so we basically did:
> reject when dlc dai_name is valid, but not match the dai name

So it is, but this still looks like the wrong thing - it'll match on an
empty DAI name over an explicit match on the driver name which seems
like it's the wrong way round.

> 
> and my patch is:
> accept when dlc dai_name is invalid
> accept when match dai name
> accept when match dai driver name(only when it is valid)
> 
> so it's a "if (a && b) reject" to "if (!a || !b) accept" case..
> 
> > 
> > > +			if (dai->driver->name &&
> > > +			    !strcmp(dai->driver->name, dlc->dai_name))
> > 
> 
>

[toc] | [prev] | [next] | [standalone]

#1718798

Fromjeffy <jeffy.chen@rock-chips.com>
Date2017-08-24 05:30 +0200
Message-ID<uhR0d-Y3-9@gated-at.bofh.it>
In reply to#1718252
Hi Mark,

On 08/23/2017 07:06 PM, Mark Brown wrote:
> On Wed, Aug 23, 2017 at 07:42:55AM +0800, jeffy wrote:
>
>> >i think the original check is allowing NULL dlc dai_name to be a match...
>> >so we basically did:
>> >reject when dlc dai_name is valid, but not match the dai name
> So it is, but this still looks like the wrong thing - it'll match on an
> empty DAI name over an explicit match on the driver name which seems
> like it's the wrong way round.
>
sorry, i don't know much about asoc, so i just trying to keep the 
original conditions and add the new one on it :)

the original one is:
if (dlc->dai_name && strcmp(dai->name, dlc->dai_name))
    continue;

and i was trying to do something like:
if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
     && (!dai->driver->name || strcmp(dai->driver->name, dlc->dai_name)))
    continue;

which is add an accept case for: dai driver name is valid and matches 
the dai name we are looking for...

[toc] | [prev] | [next] | [standalone]

#1719113

FromMark Brown <broonie@kernel.org>
Date2017-08-24 12:20 +0200
Message-ID<uhXp0-563-11@gated-at.bofh.it>
In reply to#1718798

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 24, 2017 at 11:29:42AM +0800, jeffy wrote:

> and i was trying to do something like:
> if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
>     && (!dai->driver->name || strcmp(dai->driver->name, dlc->dai_name)))
>    continue;

> which is add an accept case for: dai driver name is valid and matches the
> dai name we are looking for...

Writing it as one if statement would at least be clearer.  I can't
remember the patch you proposed at this point but the above looks
plausible.

[toc] | [prev] | [next] | [standalone]

#1719147

Fromjeffy <jeffy.chen@rock-chips.com>
Date2017-08-24 12:50 +0200
Message-ID<uhXS2-5i7-13@gated-at.bofh.it>
In reply to#1719113
hi Mark,

On 08/24/2017 06:18 PM, Mark Brown wrote:
> On Thu, Aug 24, 2017 at 11:29:42AM +0800, jeffy wrote:
>
>> and i was trying to do something like:
>> if (dlc->dai_name && strcmp(dai->name, dlc->dai_name)
>>      && (!dai->driver->name || strcmp(dai->driver->name, dlc->dai_name)))
>>     continue;
>
>> which is add an accept case for: dai driver name is valid and matches the
>> dai name we are looking for...
>
> Writing it as one if statement would at least be clearer.  I can't
> remember the patch you proposed at this point but the above looks
> plausible.
right, i've already post a v3 for that ;)
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web