[vlc-devel] [PATCH] oss: fix device selection

[Sean McGovern]

Hi Rémi,

Thanks for the review -- I will fix this and resubmit later today.

As to ai.devnode ever being NULL -- the old module would have been equally susceptible to that. Empty result from the ioctl() call => buggered kernel module => get what you deserve. I can definitely check the result of realpath() though, and realize I should have done that in the first place since it does allocate memory.

Again thanks for checking,

-- Sean McG.
-----Original Message-----
From: Rémi Denis-Courmont <remi at remlab.net>
Sender: "vlc-devel" <vlc-devel-bounces at videolan.org>Date: Tue, 10 Feb 2015 12:10:18 
To: Mailing list for VLC media player developers<vlc-devel at videolan.org>
Reply-To: Mailing list for VLC media player developers <vlc-devel at videolan.org>
Subject: Re: [vlc-devel] [PATCH] oss: fix device selection

Le 2015-02-10 05:42, Sean McGovern a écrit :
> ---
>  modules/audio_output/oss.c |   31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/modules/audio_output/oss.c b/modules/audio_output/oss.c
> index 1afd03d..3803b32 100644
> --- a/modules/audio_output/oss.c
> +++ b/modules/audio_output/oss.c
> @@ -91,20 +91,14 @@ static int Start (audio_output_t *aout,
> audio_sample_format_t *restrict fmt)
>      aout_sys_t* sys = aout->sys;
>
>      /* Open the device */
> -    const char *device = sys->device;
> -    if (device == NULL)
> -        device = getenv ("OSS_AUDIODEV");
> -    if (device == NULL)
> -        device = "/dev/dsp";
> -
> -    int fd = vlc_open (device, O_WRONLY);
> +    int fd = vlc_open (sys->device, O_WRONLY);
>      if (fd == -1)
>      {
> -        msg_Err (aout, "cannot open OSS device %s: %s", device,
> +        msg_Err (aout, "cannot open OSS device %s: %s", sys->device,
>                   vlc_strerror_c(errno));
>          return VLC_EGENERIC;
>      }
> -    msg_Dbg (aout, "using OSS device: %s", device);
> +    msg_Dbg (aout, "using OSS device: %s", sys->device);
>
>      /* Select audio format */
>      int format;
> @@ -352,7 +346,11 @@ static int DevicesEnum (audio_output_t *aout)
>          if (!ai.enabled)
>              continue;
>
> -        aout_HotplugReport (aout, ai.devnode, ai.name);
> +        /* the device ID may be a symlink -- canonicalize it */
> +        char *devnode = realpath(ai.devnode, NULL);
> +        aout_HotplugReport (aout, devnode, ai.name);

No. aout_HotplugReport(aout, NULL, str); is undefined.

> +        free (devnode);
> +
>          n++;
>      }
>  out:
> @@ -388,7 +386,17 @@ static int Open (vlc_object_t *obj)
>          return VLC_ENOMEM;
>
>      sys->fd = -1;
> -    sys->device = var_InheritString (aout, "oss-audio-device");
> +    char *device = var_InheritString (aout, "oss-audio-device");
> +    if (device == NULL)
> +        device = getenv ("OSS_AUDIODEV");
> +    if (device == NULL)
> +        device = "/dev/dsp";

Would you _please_ review compiler warnings before you send patches?

> +
> +    /* the device ID may be a symlink -- canonicalize it */
> +    sys->device = realpath (device, NULL);
> +
> +    if (device)
> +        free (device);

Invalid, and that's why you should head to compiler warnings.

>
>      aout->sys = sys;
>      aout->start = Start;
> @@ -397,6 +405,7 @@ static int Open (vlc_object_t *obj)
>      aout_SoftVolumeInit (aout);
>
>      DevicesEnum (aout);
> +    aout_DeviceReport (aout, sys->device);
>      return VLC_SUCCESS;
>  }

-- 
Rémi Denis-Courmont
_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
https://mailman.videolan.org/listinfo/vlc-devel