[alsa-devel] [PATCH 1/6] ALSA: PCM: Remove redundant null check before kfree
kfree on a null pointer is a no-op.
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- Build tested and based on linux-next 20121115. --- sound/core/pcm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 030102c..61798f8 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -981,8 +981,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control))); kfree(runtime->hw_constraints.rules); #ifdef CONFIG_SND_PCM_XRUN_DEBUG - if (runtime->hwptr_log) - kfree(runtime->hwptr_log); + kfree(runtime->hwptr_log); #endif kfree(runtime); substream->runtime = NULL;
Return the value obtained from snd_pcm_hw_constraint_minmax() instead of -EINVAL. Silences the following smatch warning: sound/core/pcm_native.c:2003 snd_pcm_hw_constraints_complete() info: why not propagate 'err' from snd_pcm_hw_constraint_minmax() instead of -22?
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- Build tested and based on linux-next 20121115. --- sound/core/pcm_native.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c76f6375..09b4286 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2000,7 +2000,7 @@ int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream) if (runtime->dma_bytes) { err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 0, runtime->dma_bytes); if (err < 0) - return -EINVAL; + return err; }
if (!(hw->rates & (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))) {
At Wed, 21 Nov 2012 14:36:55 +0530, Sachin Kamat wrote:
Return the value obtained from snd_pcm_hw_constraint_minmax() instead of -EINVAL. Silences the following smatch warning: sound/core/pcm_native.c:2003 snd_pcm_hw_constraints_complete() info: why not propagate 'err' from snd_pcm_hw_constraint_minmax() instead of -22?
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
Thanks, applied.
Takashi
Build tested and based on linux-next 20121115.
sound/core/pcm_native.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c76f6375..09b4286 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2000,7 +2000,7 @@ int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream) if (runtime->dma_bytes) { err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 0, runtime->dma_bytes); if (err < 0)
return -EINVAL;
return err;}
if (!(hw->rates & (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))) {
-- 1.7.4.1
'substream' was dereferenced before the NULL check. Moved the check earlier to prevent NULL pointer dereference.
Cc: Sangbeom Kim sbkim73@samsung.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- Build tested and based on linux-next 20121115. --- sound/soc/samsung/dma.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c index b70964e..e59a4f4 100644 --- a/sound/soc/samsung/dma.c +++ b/sound/soc/samsung/dma.c @@ -114,17 +114,23 @@ static void dma_enqueue(struct snd_pcm_substream *substream) static void audio_buffdone(void *data) { struct snd_pcm_substream *substream = data; - struct runtime_data *prtd = substream->runtime->private_data; + struct runtime_data *prtd;
pr_debug("Entered %s\n", __func__);
+ if (substream) { + prtd = substream->runtime->private_data; + } else { + pr_err("%s: Null data received\n", __func__); + return; + } + if (prtd->state & ST_RUNNING) { prtd->dma_pos += prtd->dma_period; if (prtd->dma_pos >= prtd->dma_end) prtd->dma_pos = prtd->dma_start;
- if (substream) - snd_pcm_period_elapsed(substream); + snd_pcm_period_elapsed(substream);
spin_lock(&prtd->lock); if (!samsung_dma_has_circular()) {
On Wed, Nov 21, 2012 at 02:36:56PM +0530, Sachin Kamat wrote:
static void audio_buffdone(void *data) { struct snd_pcm_substream *substream = data;
- struct runtime_data *prtd = substream->runtime->private_data;
- struct runtime_data *prtd;
This should be a BUG_ON() or something rather than just a simple warning - the checks here are redundant, there's no way we should ever end up completing a buffer without an associated runtime.
'rt' was dereferenced before the NULL check. Moved the code after the check.
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- Build tested and based on linux-next 20121115. --- sound/usb/6fire/comm.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 6c3d531..1a75c36 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -129,12 +129,13 @@ int __devinit usb6fire_comm_init(struct sfire_chip *chip) { struct comm_runtime *rt = kzalloc(sizeof(struct comm_runtime), GFP_KERNEL); - struct urb *urb = &rt->receiver; + struct urb *urb; int ret;
if (!rt) return -ENOMEM;
+ urb = &rt->receiver; rt->serial = 1; rt->chip = chip; usb_init_urb(urb);
At Wed, 21 Nov 2012 14:36:57 +0530, Sachin Kamat wrote:
'rt' was dereferenced before the NULL check. Moved the code after the check.
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
Applied (with a minor change in the subject line). Thanks.
Takashi
Build tested and based on linux-next 20121115.
sound/usb/6fire/comm.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 6c3d531..1a75c36 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -129,12 +129,13 @@ int __devinit usb6fire_comm_init(struct sfire_chip *chip) { struct comm_runtime *rt = kzalloc(sizeof(struct comm_runtime), GFP_KERNEL);
- struct urb *urb = &rt->receiver;
struct urb *urb; int ret;
if (!rt) return -ENOMEM;
urb = &rt->receiver; rt->serial = 1; rt->chip = chip; usb_init_urb(urb);
-- 1.7.4.1
Return -ENOMEM instead of -1 to silence the following smatch warning: sound/usb/format.c:170 parse_audio_format_rates_v1() warn: returning -1 instead of -ENOMEM is sloppy
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- Build tested and based on linux-next 20121115. --- sound/usb/format.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/usb/format.c b/sound/usb/format.c index ddfef57..d033d4d 100644 --- a/sound/usb/format.c +++ b/sound/usb/format.c @@ -167,7 +167,7 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL); if (fp->rate_table == NULL) { snd_printk(KERN_ERR "cannot malloc\n"); - return -1; + return -ENOMEM; }
fp->nr_rates = 0;
At Wed, 21 Nov 2012 14:36:58 +0530, Sachin Kamat wrote:
Return -ENOMEM instead of -1 to silence the following smatch warning: sound/usb/format.c:170 parse_audio_format_rates_v1() warn: returning -1 instead of -ENOMEM is sloppy
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
There are some other places returning -1 around this. Could you fix them as well to sensitive values?
thanks,
Takashi
Build tested and based on linux-next 20121115.
sound/usb/format.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/usb/format.c b/sound/usb/format.c index ddfef57..d033d4d 100644 --- a/sound/usb/format.c +++ b/sound/usb/format.c @@ -167,7 +167,7 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL); if (fp->rate_table == NULL) { snd_printk(KERN_ERR "cannot malloc\n");
return -1;
return -ENOMEM;}
fp->nr_rates = 0;
-- 1.7.4.1
On 21 November 2012 15:15, Takashi Iwai tiwai@suse.de wrote:
At Wed, 21 Nov 2012 14:36:58 +0530, Sachin Kamat wrote:
Return -ENOMEM instead of -1 to silence the following smatch warning: sound/usb/format.c:170 parse_audio_format_rates_v1() warn: returning -1 instead of -ENOMEM is sloppy
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
There are some other places returning -1 around this. Could you fix them as well to sensitive values?
Ok. Will update and re-send this patch.
thanks,
Takashi
Build tested and based on linux-next 20121115.
sound/usb/format.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/usb/format.c b/sound/usb/format.c index ddfef57..d033d4d 100644 --- a/sound/usb/format.c +++ b/sound/usb/format.c @@ -167,7 +167,7 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL); if (fp->rate_table == NULL) { snd_printk(KERN_ERR "cannot malloc\n");
return -1;
return -ENOMEM; } fp->nr_rates = 0;-- 1.7.4.1
'w' should not be dereferenced when it is NULL.
Cc: Liam Girdwood lrg@ti.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org --- Build tested and based on linux-next 20121115. --- sound/soc/soc-dapm.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 6e35bca..5bde5d4 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3326,9 +3326,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, if (!w) { dev_err(dapm->dev, "Failed to create %s widget\n", dai->driver->playback.stream_name); + } else { + w->priv = dai; } - - w->priv = dai; dai->playback_widget = w; }
@@ -3344,9 +3344,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, if (!w) { dev_err(dapm->dev, "Failed to create %s widget\n", dai->driver->capture.stream_name); + } else { + w->priv = dai; } - - w->priv = dai; dai->capture_widget = w; }
Not sure if this patch got lost. Just a gentle reminder.
On 21 November 2012 14:36, Sachin Kamat sachin.kamat@linaro.org wrote:
'w' should not be dereferenced when it is NULL.
Cc: Liam Girdwood lrg@ti.com Cc: Mark Brown broonie@opensource.wolfsonmicro.com Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
Build tested and based on linux-next 20121115.
sound/soc/soc-dapm.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 6e35bca..5bde5d4 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3326,9 +3326,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, if (!w) { dev_err(dapm->dev, "Failed to create %s widget\n", dai->driver->playback.stream_name);
} else {w->priv = dai; }
w->priv = dai; dai->playback_widget = w; }@@ -3344,9 +3344,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm, if (!w) { dev_err(dapm->dev, "Failed to create %s widget\n", dai->driver->capture.stream_name);
} else {w->priv = dai; }
w->priv = dai; dai->capture_widget = w; }-- 1.7.4.1
On Wed, Nov 28, 2012 at 10:13:58AM +0530, Sachin Kamat wrote:
Not sure if this patch got lost. Just a gentle reminder.
Don't send contentless pings or top post. If the patch has been lost it's not helpful to have a copy covered in quotation marks.
On Wed, Nov 21, 2012 at 02:36:59PM +0530, Sachin Kamat wrote:
if (!w) { dev_err(dapm->dev, "Failed to create %s widget\n", dai->driver->playback.stream_name);
} else {w->priv = dai;
This isn't a particularly sensible fix, we should be erroring out or something rather than limping along with the rest of the function.
At Wed, 21 Nov 2012 14:36:54 +0530, Sachin Kamat wrote:
kfree on a null pointer is a no-op.
Signed-off-by: Sachin Kamat sachin.kamat@linaro.org
Thanks, applied.
Takashi
Build tested and based on linux-next 20121115.
sound/core/pcm.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 030102c..61798f8 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -981,8 +981,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control))); kfree(runtime->hw_constraints.rules); #ifdef CONFIG_SND_PCM_XRUN_DEBUG
- if (runtime->hwptr_log)
kfree(runtime->hwptr_log);
- kfree(runtime->hwptr_log);
#endif kfree(runtime); substream->runtime = NULL; -- 1.7.4.1
participants (3)
-
Mark Brown -
Sachin Kamat -
Takashi Iwai