| Message ID | CAGFh027Yf8wQxsxNkMDcN8cSZGu38CviYN=KSLZWabrZsOaV6g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ioplug: Check for callback error codes | expand |
On Mon, 23 Aug 2021 13:46:37 +0200, Arkadiusz Bokowy wrote: > > Signed-off-by: Arkadiusz Bokowy <arkadiusz.bokowy@gmail.com> The lack of the patch description is always a sign of a bad patch. Please put more information here, especially why this change is required. And the whole patch seems malformed and can't be applied. Please fix your mailer setup. About the code change: > /* update the hw pointer */ > /* called in lock */ > -static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) > +static int snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) > { > ioplug_priv_t *io = pcm->private_data; > snd_pcm_sframes_t hw; > @@ -85,7 +85,9 @@ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) > snd_pcm_ioplug_drop(pcm); > else > io->data->state = SNDRV_PCM_STATE_XRUN; > + return -EPIPE; If xrun happens during the draining, it's rather handled as successfully drained, hence better to return 0 there. > @@ -898,13 +906,14 @@ static void clear_io_params(ioplug_priv_t *io) > static int snd_pcm_ioplug_close(snd_pcm_t *pcm) > { > ioplug_priv_t *io = pcm->private_data; > + int err = 0; > > clear_io_params(io); > if (io->data->callback->close) > - io->data->callback->close(io->data); > + err = io->data->callback->close(io->data); > free(io); > > - return 0; > + return err; This is dangerous. It may leave a error state while the resources have been already released. Then application cannot do anything after that. If we really want to check the return value, the resource releases should be done after that point, so that application may call the close again. thanks, Takashi
diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c index 98184398..c96104e9 100644 --- a/src/pcm/pcm_ioplug.c +++ b/src/pcm/pcm_ioplug.c @@ -54,7 +54,7 @@ static int snd_pcm_ioplug_poll_revents(snd_pcm_t *pcm, struct pollfd *pfds, unsi /* update the hw pointer */ /* called in lock */ -static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) +static int snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) { ioplug_priv_t *io = pcm->private_data;
Signed-off-by: Arkadiusz Bokowy <arkadiusz.bokowy@gmail.com> --- src/pcm/pcm_ioplug.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) snd_pcm_sframes_t hw; @@ -85,7 +85,9 @@ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm) snd_pcm_ioplug_drop(pcm); else io->data->state = SNDRV_PCM_STATE_XRUN; + return -EPIPE; } + return 0; } static int snd_pcm_ioplug_info(snd_pcm_t *pcm, snd_pcm_info_t *info) @@ -115,7 +117,10 @@ static int snd_pcm_ioplug_delay(snd_pcm_t *pcm, snd_pcm_sframes_t *delayp) io->data->callback->delay) return io->data->callback->delay(io->data, delayp); else { - snd_pcm_ioplug_hw_ptr_update(pcm); + int err; + err = snd_pcm_ioplug_hw_ptr_update(pcm); + if (err < 0) + return err; *delayp = snd_pcm_mmap_delay(pcm); } return 0; @@ -499,11 +504,14 @@ static int snd_pcm_ioplug_start(snd_pcm_t *pcm) static int snd_pcm_ioplug_drop(snd_pcm_t *pcm) { ioplug_priv_t *io = pcm->private_data; + int err; if (io->data->state == SND_PCM_STATE_OPEN) return -EBADFD; - io->data->callback->stop(io->data); + err = io->data->callback->stop(io->data); + if (err < 0) + return err; gettimestamp(&io->trigger_tstamp, pcm->tstamp_type); io->data->state = SND_PCM_STATE_SETUP; @@ -625,7 +633,7 @@ static int snd_pcm_ioplug_resume(snd_pcm_t *pcm) ioplug_priv_t *io = pcm->private_data; if (io->data->callback->resume) - io->data->callback->resume(io->data); + return io->data->callback->resume(io->data); return 0; } @@ -898,13 +906,14 @@ static void clear_io_params(ioplug_priv_t *io) static int snd_pcm_ioplug_close(snd_pcm_t *pcm) { ioplug_priv_t *io = pcm->private_data; + int err = 0; clear_io_params(io); if (io->data->callback->close) - io->data->callback->close(io->data); + err = io->data->callback->close(io->data); free(io); - return 0; + return err; } static const snd_pcm_ops_t snd_pcm_ioplug_ops = {