Hi Rickard,
On 20.05.2014 21:12, Rickard Strandqvist wrote:
Hi Tomasz
What I based my patch on is really because of this line: if (substream) snd_pcm_period_elapsed(substream);
Boojin Kim thought that this was needed, if this is true anymore..? I have not been able to immerse myself so much in all patches. I'm working on about 100 similar patches.
To me having NULL as either data argument of buffer done callback or private_data would be a serious driver bug and IMHO it's better to let it crash with a NULL pointer dereference to let someone notice than mask it by adding a condition.
Still, I'm not too experienced with ALSA and ASoC, so I might be wrong. Mark, what do you think about this?
Best regards, Tomasz
344b4c48 sound/soc/samsung/dma.c (Boojin Kim 2011-09-02 09:44:43 +0900 123) prtd->dma_pos += prtd->dma_period; 344b4c48 sound/soc/samsung/dma.c (Boojin Kim 2011-09-02 09:44:43 +0900 124) if (prtd->dma_pos >= prtd->dma_end) 344b4c48 sound/soc/samsung/dma.c (Boojin Kim 2011-09-02 09:44:43 +0900 125) prtd->dma_pos = prtd->dma_start; 5111c075 sound/soc/s3c24xx/s3c24xx-pcm.c (Mark Brown 2008-04-30 17:19:32 +0200 126) 344b4c48 sound/soc/samsung/dma.c (Boojin Kim 2011-09-02 09:44:43 +0900 127) if (substream) 344b4c48 sound/soc/samsung/dma.c (Boojin Kim 2011-09-02 09:44:43 +0900 128) snd_pcm_period_elapsed(substream); c0f41bb1 sound/soc/s3c24xx/s3c24xx-pcm.c (Ben Dooks 2007-02-14 13:20:03 +0100 129) 344b4c48 sound/soc/samsung/dma.c (Boojin Kim 2011-09-02 09:44:43 +0900 130) spin_lock(&prtd->lock);
Best regards Rickard Strandqvist
2014-05-19 12:52 GMT+02:00 Tomasz Figa tomasz.figa@gmail.com:
Hi Rickard,
On 15.05.2014 23:57, Rickard Strandqvist wrote:
There is otherwise a risk of a possible null pointer dereference.
Was largely found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist rickard_strandqvist@spectrumdigital.se
sound/soc/samsung/dma.c | 10 ++++++---- 1 fil ändrad, 6 tillägg(+), 4 borttagningar(-)
diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c index dc09b71..b1f6757 100644 --- a/sound/soc/samsung/dma.c +++ b/sound/soc/samsung/dma.c @@ -115,17 +115,19 @@ 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 = NULL; pr_debug("Entered %s\n", __func__);
if (prtd->state & ST_RUNNING) {
if(substream)prtd = substream->runtime->private_data;if (prtd && 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()) {Can you find a path (or use case, if possible) on which any of affected pointers will be NULL?
Best regards, Tomasz