Re: [PATCH] ao/alsa: Plug device leak if init() fails

On 31.08.2017, at 01:12, Ville Syrjälä <[email protected]> wrote:

> Close the device instead of leaking it when init() fails. Otherwise
> subsequent attempts at using the device are going to fail if the
> device doesn't support mixing multiple streams.

I suspect this really should be using "goto error".
I haven't reviewed if uninit is carefully written to always be safe to call on a half-initialized module.


> ---
> libao2/ao_alsa.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> 
> diff --git a/libao2/ao_alsa.c b/libao2/ao_alsa.c
> index 36dcc1136926..2ada64d3bec8 100644
> --- a/libao2/ao_alsa.c
> +++ b/libao2/ao_alsa.c
> @@ -508,10 +508,12 @@ static int init(int rate_hz, int channels, int format, int flags)
>        mp_msg(MSGT_AO,MSGL_INFO,MSGTR_AO_ALSA_OpenInNonblockModeFailed);
>        if ((err = try_open_device(alsa_device, 0, isac3)) < 0) {
>          mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_PlaybackOpenError, snd_strerror(err));
> +          uninit(1);
>          return 0;
>        }
>      } else {
>        mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_PlaybackOpenError, snd_strerror(err));
> +        uninit(1);
>        return 0;
>      }
>    }
> @@ -530,6 +532,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>    {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToGetInitialParameters,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
> 
> @@ -538,6 +541,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>       if (err < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetAccessType,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
> 
> @@ -562,6 +566,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>    {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetFormat,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
> 
> @@ -570,6 +575,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>    {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetChannels,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
> 
> @@ -582,6 +588,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>    {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToDisableResampling,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
> #endif
> @@ -591,6 +598,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>         {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetSamplerate2,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>         }
> 
> @@ -603,6 +611,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>      {
>        mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetBufferTimeNear,
>           snd_strerror(err));
> +        uninit(1);
>        return 0;
>      }
> 
> @@ -610,6 +619,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>                              &alsa_fragcount, NULL)) < 0) {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetPeriods,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
> 
> @@ -618,6 +628,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>    {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetHwParameters,
>         snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
>       // end setting hw-params
> @@ -627,6 +638,7 @@ static int init(int rate_hz, int channels, int format, int flags)
>       if ((err = snd_pcm_hw_params_get_buffer_size(alsa_hwparams, &bufsize)) < 0)
>    {
>      mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToGetBufferSize, snd_strerror(err));
> +      uninit(1);
>      return 0;
>    }
>       else {
> @@ -636,6 +648,7 @@ static int init(int rate_hz, int channels, int format, int flags)
> 
>       if ((err = snd_pcm_hw_params_get_period_size(alsa_hwparams, &chunk_size, NULL)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToGetPeriodSize, snd_strerror(err));
> +    uninit(1);
>    return 0;
>       } else {
>    mp_msg(MSGT_AO,MSGL_V,"alsa-init: got period size %li\n", chunk_size);
> @@ -646,12 +659,14 @@ static int init(int rate_hz, int channels, int format, int flags)
>       if ((err = snd_pcm_sw_params_current(alsa_handler, alsa_swparams)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToGetSwParameters,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
> #if SND_LIB_VERSION >= 0x000901
>       if ((err = snd_pcm_sw_params_get_boundary(alsa_swparams, &boundary)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToGetBoundary,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
> #else
> @@ -661,12 +676,14 @@ static int init(int rate_hz, int channels, int format, int flags)
>       if ((err = snd_pcm_sw_params_set_start_threshold(alsa_handler, alsa_swparams, chunk_size)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetStartThreshold,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
>       /* disable underrun reporting */
>       if ((err = snd_pcm_sw_params_set_stop_threshold(alsa_handler, alsa_swparams, boundary)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetStopThreshold,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
> #if SND_LIB_VERSION >= 0x000901
> @@ -674,12 +691,14 @@ static int init(int rate_hz, int channels, int format, int flags)
>       if ((err = snd_pcm_sw_params_set_silence_size(alsa_handler, alsa_swparams, boundary)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToSetSilenceSize,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
> #endif
>       if ((err = snd_pcm_sw_params(alsa_handler, alsa_swparams)) < 0) {
>    mp_msg(MSGT_AO,MSGL_ERR,MSGTR_AO_ALSA_UnableToGetSwParameters,
>           snd_strerror(err));
> +    uninit(1);
>    return 0;
>       }
>       /* end setting sw-params */
> -- 
> 2.13.0
> 
> _______________________________________________
> MPlayer-dev-eng mailing list
> [email protected]
> https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng