[PATCH 1/2] ioplug: Check for callback error codes

[PATCH 1/2] ioplug: Check for callback error codes

[Arkadiusz Bokowy]

Signed-off-by: Arkadiusz Bokowy <arkadiusz.bokowy@gmail.com>
---
 src/pcm/pcm_ioplug.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

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;
  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 = {
-- 
2.31.1

Re: [PATCH 1/2] ioplug: Check for callback error codes

[Takashi Iwai]

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