From: Peter Rosin <[email protected]>
If a snd_soc_card has any DAPM routes when it calls
snd_soc_of_parse_audio_routing, those are clobbered without this change.
Signed-off-by: Peter Rosin <[email protected]>
---
sound/soc/soc-core.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b60ff56ebc0f..377db486852e 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4585,7 +4585,7 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
const char *propname)
{
struct device_node *np = card->dev->of_node;
- int num_routes;
+ int num_routes, old_routes;
struct snd_soc_dapm_route *routes;
int i, ret;
@@ -4603,7 +4603,9 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
return -EINVAL;
}
- routes = devm_kzalloc(card->dev, num_routes * sizeof(*routes),
+ old_routes = card->num_dapm_routes;
+ routes = devm_kzalloc(card->dev,
+ (old_routes + num_routes) * sizeof(*routes),
GFP_KERNEL);
if (!routes) {
dev_err(card->dev,
@@ -4611,9 +4613,11 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
return -EINVAL;
}
+ memcpy(routes, card->dapm_routes, old_routes * sizeof(*routes));
+
for (i = 0; i < num_routes; i++) {
ret = of_property_read_string_index(np, propname,
- 2 * i, &routes[i].sink);
+ 2 * i, &routes[old_routes + i].sink);
if (ret) {
dev_err(card->dev,
"ASoC: Property '%s' index %d could not be read: %d\n",
@@ -4621,7 +4625,7 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
return -EINVAL;
}
ret = of_property_read_string_index(np, propname,
- (2 * i) + 1, &routes[i].source);
+ (2 * i) + 1, &routes[old_routes + i].source);
if (ret) {
dev_err(card->dev,
"ASoC: Property '%s' index %d could not be read: %d\n",
@@ -4630,7 +4634,7 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
}
}
- card->num_dapm_routes = num_routes;
+ card->num_dapm_routes += num_routes;
card->dapm_routes = routes;
return 0;
--
1.7.10.4
On Thu, Nov 27, 2014 at 10:02:42PM +0100, Peter Rosin wrote:
> - routes = devm_kzalloc(card->dev, num_routes * sizeof(*routes),
> + old_routes = card->num_dapm_routes;
> + routes = devm_kzalloc(card->dev,
> + (old_routes + num_routes) * sizeof(*routes),
> GFP_KERNEL);
> if (!routes) {
> dev_err(card->dev,
> @@ -4611,9 +4613,11 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
> return -EINVAL;
> }
>
> + memcpy(routes, card->dapm_routes, old_routes * sizeof(*routes));
> +
Aren't we open coding krealloc() here?
Mark Brown wrote:
> On Thu, Nov 27, 2014 at 10:02:42PM +0100, Peter Rosin wrote:
>
> > - routes = devm_kzalloc(card->dev, num_routes * sizeof(*routes),
> > + old_routes = card->num_dapm_routes;
> > + routes = devm_kzalloc(card->dev,
> > + (old_routes + num_routes) * sizeof(*routes),
> > GFP_KERNEL);
> > if (!routes) {
> > dev_err(card->dev,
> > @@ -4611,9 +4613,11 @@ int snd_soc_of_parse_audio_routing(struct
> snd_soc_card *card,
> > return -EINVAL;
> > }
> >
> > + memcpy(routes, card->dapm_routes, old_routes * sizeof(*routes));
> > +
>
> Aren't we open coding krealloc() here?
I don't think krealloc() is appropriate since we don't know where the memory
comes from. The typical use I see is that the card struct is initialized as:
static const struct snd_soc_dapm_route foo_intercon[] = {
{ "MUX1", "Loop", "IN1" },
{ "MUX1", "Mixer", "MIX1" },
{ "MIX1", NULL, "DAC" },
{ "MIX1", "IN1 Switch", "IN1" },
{ "Line Out Jack", NULL, "MUX1" },
};
static struct snd_soc_card foo_card = {
.name = "foo",
.owner = THIS_MODULE,
.dapm_routes = foo_intercon,
.num_dapm_routes = ARRAY_SIZE(foo_intercon),
/* etc */
};
If that's the case, krealloc() seems dead wrong. On the other hand, if
snd_soc_of_parse_audio_routing() were to be called many times, a lot
of devm_kzalloc()ed memory would be kept dangling. I don't expect a card
to call snd_soc_of_parse_audio_routing() more than once though...
Cheers,
Peter
On Thu, Nov 27, 2014 at 10:02:42PM +0100, Peter Rosin wrote:
> From: Peter Rosin <[email protected]>
>
> If a snd_soc_card has any DAPM routes when it calls
> snd_soc_of_parse_audio_routing, those are clobbered without this change.
Applied, thanks.