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