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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 23 Aug 2021 13:46:37 +0200,
Arkadiusz Bokowy wrote:
> 
> Signed-off-by: Arkadiusz Bokowy <arkadiusz.bokowy@xxxxxxxxx>

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



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux