[alsa-devel] [PATCH] pcm_plugin: fix appl pointer not correct when mmap_commit() return error
When snd_pcm_mmap_commit() return error, the appl pointer is also updated. which cause the avail_update()'s result wrong. This patch move the snd_pcm_mmap_appl_forward() to the place when snd_pcm_mmap_commit() is successfully returned.
Signed-off-by: Shengjiu Wang shengjiu.wang@freescale.com --- src/pcm/pcm_plugin.c | 48 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c index d007e8c..940491d 100644 --- a/src/pcm/pcm_plugin.c +++ b/src/pcm/pcm_plugin.c @@ -279,18 +279,22 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm, return -EPIPE; } snd_atomic_write_begin(&plugin->watom); - snd_pcm_mmap_appl_forward(pcm, frames); result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames); if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) { snd_pcm_sframes_t res; res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result); - if (res < 0) + if (res < 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? (snd_pcm_sframes_t)xfer : res; + } frames -= res; } - snd_atomic_write_end(&plugin->watom); - if (result <= 0) + if (result <= 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? (snd_pcm_sframes_t)xfer : result; + } + snd_pcm_mmap_appl_forward(pcm, frames); + snd_atomic_write_end(&plugin->watom); offset += frames; xfer += frames; size -= frames; @@ -325,19 +329,23 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm, return -EPIPE; } snd_atomic_write_begin(&plugin->watom); - snd_pcm_mmap_appl_forward(pcm, frames); result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames); if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) { snd_pcm_sframes_t res; res = plugin->undo_read(slave, areas, offset, frames, slave_frames - result); - if (res < 0) + if (res < 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? (snd_pcm_sframes_t)xfer : res; + } frames -= res; } - snd_atomic_write_end(&plugin->watom); - if (result <= 0) + if (result <= 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? (snd_pcm_sframes_t)xfer : result; + } + snd_pcm_mmap_appl_forward(pcm, frames); + snd_atomic_write_end(&plugin->watom); offset += frames; xfer += frames; size -= frames; @@ -423,19 +431,23 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm, frames = plugin->write(pcm, areas, appl_offset, frames, slave_areas, slave_offset, &slave_frames); snd_atomic_write_begin(&plugin->watom); - snd_pcm_mmap_appl_forward(pcm, frames); result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames); - snd_atomic_write_end(&plugin->watom); if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) { snd_pcm_sframes_t res; res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result); - if (res < 0) + if (res < 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? xfer : res; + } frames -= res; } - if (result <= 0) + if (result <= 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? xfer : result; + } + snd_pcm_mmap_appl_forward(pcm, frames); + snd_atomic_write_end(&plugin->watom); if (frames == cont) appl_offset = 0; else @@ -490,19 +502,23 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm) frames = (plugin->read)(pcm, areas, hw_offset, frames, slave_areas, slave_offset, &slave_frames); snd_atomic_write_begin(&plugin->watom); - snd_pcm_mmap_hw_forward(pcm, frames); result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames); - snd_atomic_write_end(&plugin->watom); if (result > 0 && (snd_pcm_uframes_t)result != slave_frames) { snd_pcm_sframes_t res; res = plugin->undo_read(slave, areas, hw_offset, frames, slave_frames - result); - if (res < 0) + if (res < 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? (snd_pcm_sframes_t)xfer : res; + } frames -= res; } - if (result <= 0) + if (result <= 0) { + snd_atomic_write_end(&plugin->watom); return xfer > 0 ? (snd_pcm_sframes_t)xfer : result; + } + snd_pcm_mmap_hw_forward(pcm, frames); + snd_atomic_write_end(&plugin->watom); if (frames == cont) hw_offset = 0; else
On Wed, 06 Apr 2016 13:02:12 +0200, Shengjiu Wang wrote:
When snd_pcm_mmap_commit() return error, the appl pointer is also updated. which cause the avail_update()'s result wrong. This patch move the snd_pcm_mmap_appl_forward() to the place when snd_pcm_mmap_commit() is successfully returned.
Signed-off-by: Shengjiu Wang shengjiu.wang@freescale.com
Thanks, applied. I also cleaned up the error paths in the patch below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] pcm: Clean up error paths in snd_pcm_plugin_*() helpers
Minor code refactoring to unify the error return paths.
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/pcm/pcm_plugin.c | 67 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 22 deletions(-)
diff --git a/src/pcm/pcm_plugin.c b/src/pcm/pcm_plugin.c index 940491dbc84b..8527783c3569 100644 --- a/src/pcm/pcm_plugin.c +++ b/src/pcm/pcm_plugin.c @@ -276,7 +276,8 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm, if (CHECK_SANITY(slave_frames > snd_pcm_mmap_playback_avail(slave))) { SNDMSG("write overflow %ld > %ld", slave_frames, snd_pcm_mmap_playback_avail(slave)); - return -EPIPE; + err = -EPIPE; + goto error; } snd_atomic_write_begin(&plugin->watom); result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames); @@ -284,14 +285,14 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm, snd_pcm_sframes_t res; res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result); if (res < 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : res; + err = res; + goto error_atomic; } frames -= res; } if (result <= 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : result; + err = result; + goto error_atomic; } snd_pcm_mmap_appl_forward(pcm, frames); snd_atomic_write_end(&plugin->watom); @@ -300,6 +301,11 @@ static snd_pcm_sframes_t snd_pcm_plugin_write_areas(snd_pcm_t *pcm, size -= frames; } return (snd_pcm_sframes_t)xfer; + + error_atomic: + snd_atomic_write_end(&plugin->watom); + error: + return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; }
static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm, @@ -311,6 +317,7 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm, snd_pcm_t *slave = plugin->gen.slave; snd_pcm_uframes_t xfer = 0; snd_pcm_sframes_t result; + int err; while (size > 0) { snd_pcm_uframes_t frames = size; @@ -326,7 +333,8 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm, if (CHECK_SANITY(slave_frames > snd_pcm_mmap_capture_avail(slave))) { SNDMSG("read overflow %ld > %ld", slave_frames, snd_pcm_mmap_playback_avail(slave)); - return -EPIPE; + err = -EPIPE; + goto error; } snd_atomic_write_begin(&plugin->watom); result = snd_pcm_mmap_commit(slave, slave_offset, slave_frames); @@ -335,14 +343,14 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm, res = plugin->undo_read(slave, areas, offset, frames, slave_frames - result); if (res < 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : res; + err = res; + goto error_atomic; } frames -= res; } if (result <= 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : result; + err = result; + goto error_atomic; } snd_pcm_mmap_appl_forward(pcm, frames); snd_atomic_write_end(&plugin->watom); @@ -351,6 +359,11 @@ static snd_pcm_sframes_t snd_pcm_plugin_read_areas(snd_pcm_t *pcm, size -= frames; } return (snd_pcm_sframes_t)xfer; + + error_atomic: + snd_atomic_write_end(&plugin->watom); + error: + return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; }
@@ -401,6 +414,7 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm, snd_pcm_uframes_t appl_offset; snd_pcm_sframes_t slave_size; snd_pcm_sframes_t xfer; + int err;
if (pcm->stream == SND_PCM_STREAM_CAPTURE) { snd_atomic_write_begin(&plugin->watom); @@ -421,11 +435,10 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm, snd_pcm_uframes_t slave_offset; snd_pcm_uframes_t slave_frames = ULONG_MAX; snd_pcm_sframes_t result; - int err;
err = snd_pcm_mmap_begin(slave, &slave_areas, &slave_offset, &slave_frames); if (err < 0) - return xfer > 0 ? xfer : err; + goto error; if (frames > cont) frames = cont; frames = plugin->write(pcm, areas, appl_offset, frames, @@ -437,14 +450,14 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm, res = plugin->undo_write(pcm, slave_areas, slave_offset + result, slave_frames, slave_frames - result); if (res < 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? xfer : res; + err = res; + goto error_atomic; } frames -= res; } if (result <= 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? xfer : result; + err = result; + goto error_atomic; } snd_pcm_mmap_appl_forward(pcm, frames); snd_atomic_write_end(&plugin->watom); @@ -461,6 +474,11 @@ snd_pcm_plugin_mmap_commit(snd_pcm_t *pcm, return -EPIPE; } return xfer; + + error_atomic: + snd_atomic_write_end(&plugin->watom); + error: + return xfer > 0 ? xfer : err; }
static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm) @@ -468,6 +486,7 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm) snd_pcm_plugin_t *plugin = pcm->private_data; snd_pcm_t *slave = plugin->gen.slave; snd_pcm_sframes_t slave_size; + int err;
slave_size = snd_pcm_avail_update(slave); if (pcm->stream == SND_PCM_STREAM_CAPTURE && @@ -492,11 +511,10 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm) snd_pcm_uframes_t slave_offset; snd_pcm_uframes_t slave_frames = ULONG_MAX; snd_pcm_sframes_t result; - int err;
err = snd_pcm_mmap_begin(slave, &slave_areas, &slave_offset, &slave_frames); if (err < 0) - return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; + goto error; if (frames > cont) frames = cont; frames = (plugin->read)(pcm, areas, hw_offset, frames, @@ -508,14 +526,14 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm) res = plugin->undo_read(slave, areas, hw_offset, frames, slave_frames - result); if (res < 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : res; + err = res; + goto error_atomic; } frames -= res; } if (result <= 0) { - snd_atomic_write_end(&plugin->watom); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : result; + err = result; + goto error_atomic; } snd_pcm_mmap_hw_forward(pcm, frames); snd_atomic_write_end(&plugin->watom); @@ -528,6 +546,11 @@ static snd_pcm_sframes_t snd_pcm_plugin_avail_update(snd_pcm_t *pcm) xfer += frames; } return (snd_pcm_sframes_t)xfer; + + error_atomic: + snd_atomic_write_end(&plugin->watom); + error: + return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; } }
participants (2)
-
Shengjiu Wang
-
Takashi Iwai