| Message ID | 1417122162-8122-1-git-send-email-peda@lysator.liu.se (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f8781db8aeb18d34edb191bfed9c0a68affaaa74 |
| Headers | show |
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 <peda@axentia.se> > > 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.
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;