On Nov 9 2017 11:11, Kuninori Morimoto wrote:
Hi Takashi-san, Mark
snd_pcm_running() is using "substream" and "substream->runtime"
pointer, no check.
These patches adds its check in function,
and removes duplicate checks from each drivers.
Not super important, but can be cleanup
Kuninori Morimoto (6):
ALSA: pcm: check parameter on snd_pcm_running()
ALSA: pdaudiocf: remove unneeded check for snd_pcm_running()
ASoC: dwc: remove unneeded check for snd_pcm_running()
ASoC: omap-hdmi-audio: remove unneeded check for snd_pcm_running()
ASoC: xtfpga-i2s: remove unneeded check for snd_pcm_running()
ASoC: rsnd: remove unneeded check for snd_pcm_running()
include/sound/pcm.h | 3 +++
sound/pcmcia/pdaudiocf/pdaudiocf_irq.c | 2 +-
sound/soc/dwc/dwc-pcm.c | 2 +-
sound/soc/omap/omap-hdmi-audio.c | 3 +--
sound/soc/sh/rcar/core.c | 5 +----
sound/soc/xtensa/xtfpga-i2s.c | 4 ++--
6 files changed, 9 insertions(+), 10 deletions(-)
This is a bad direction. I exactly oppose to your idea.
> include/sound/pcm.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 24febf9..a8e49f5 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -664,6 +664,9 @@ void snd_pcm_stream_unlock_irqrestore(struct
snd_pcm_substream *substream,
> */
> static inline int snd_pcm_running(struct snd_pcm_substream *substream)
> {
> + if (!substream || !substream->runtime)
> + return 0;
> +
> return (substream->runtime->status->state ==
SNDRV_PCM_STATE_RUNNING ||
> (substream->runtime->status->state ==
SNDRV_PCM_STATE_DRAINING &&
> substream->stream == SNDRV_PCM_STREAM_PLAYBACK));
In a view of 'design by contract', this function has a pre-condition
that a given argument should not be NULL. Callers _should_ guarantee it
to keep semantics of this function.
Your idea appends the duty of callers to this function. This causes a
semantical contradiction. If it were something to bring kernel
corruption such as BUG_ON(), the original design would be kept. When
substream is NULL, it's a bug of drivers in adding PCM components. When
runtime is NULL, it's a bug of ALSA PCM core in handling open system call.
Regards
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
