Patchwork Workaround for broken OSS_GETVERSION on FreeBSD, part two

login
register
mail settings
Submitter Juergen Lock
Date Jan. 9, 2010, 9:05 p.m.
Message ID <20100109210507.GA92510@triton8.kn-bremen.de>
Download mbox | patch
Permalink /patch/42564/
State New
Headers show

Comments

Juergen Lock - Jan. 9, 2010, 9:05 p.m.
Turns out on those versions of FreeBSD (>= 7.x) that know OSS_GETVERSION
the ioctl doesn't actually work yet (except in the Linuxolator), so if
building on FreeBSD fall back to using SOUND_VERSION as defined in
<sys/soundcard.h> (which atm is 0x040000) if the ioctl is defined but
fails.

 Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
malc - Jan. 9, 2010, 10:40 p.m.
On Sat, 9 Jan 2010, Juergen Lock wrote:

> Turns out on those versions of FreeBSD (>= 7.x) that know OSS_GETVERSION
> the ioctl doesn't actually work yet (except in the Linuxolator), so if
> building on FreeBSD fall back to using SOUND_VERSION as defined in
> <sys/soundcard.h> (which atm is 0x040000) if the ioctl is defined but
> fails.

I've changed the code in the meantime, so this wont apply. Also this
is wrong version is dynamic by nature, if someone runs qemu on FreeBSD
where SOUND_VERSION is less than 4, things will get ugly.

In the new code i mentioned one can opt for not considering POLICY
failure a hard error and proceed with a SETFRAGMENT path, which is
what i would prefer to see on FreeBSD.

> 
>  Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
> 
> --- a/audio/ossaudio.c
> +++ b/audio/ossaudio.c
> @@ -289,8 +289,25 @@ static int oss_open (int in, struct oss_
>  
>  #ifdef USE_DSP_POLICY
>      if (ioctl (fd, OSS_GETVERSION, &version)) {
> +#ifdef __FreeBSD__
> +        /*
> +	 * Looks like atm (20100109) FreeBSD knows OSS_GETVERSION
> +	 * since 7.x, but currently only on the mixer device (or in
> +	 * the Linuxolator), and in the native version that part of
> +	 * the code is in fact never reached so the ioctl fails anyway.
> +	 * But since it just (attempts to) return SOUND_VERSION as
> +	 * defined in <sys/soundcard.h> we can fall back to getting it
> +	 * from there.  (On FreeBSD, /usr/include is part of the base
> +	 * system which is assumed to be always in sync with the kernel.)
> +	 * XXX What if the user runs the original OSS from ports instead
> +	 * of FreeBSD's own code?  Well I hope we can assume the ioctl
> +	 * works there... :)
> +	 */
> +        version = SOUND_VERSION;
> +#else
>          oss_logerr2 (errno, typ, "Failed to get OSS version\n");
>          version = 0;
> +#endif
>      }
>  
>      if (conf.debug) {
> 
>
Juergen Lock - Jan. 10, 2010, 6:57 p.m.
On Sun, Jan 10, 2010 at 01:40:18AM +0300, malc wrote:
> On Sat, 9 Jan 2010, Juergen Lock wrote:
> 
> > Turns out on those versions of FreeBSD (>= 7.x) that know OSS_GETVERSION
> > the ioctl doesn't actually work yet (except in the Linuxolator), so if
> > building on FreeBSD fall back to using SOUND_VERSION as defined in
> > <sys/soundcard.h> (which atm is 0x040000) if the ioctl is defined but
> > fails.
> 
> I've changed the code in the meantime, so this wont apply. Also this
> is wrong version is dynamic by nature, if someone runs qemu on FreeBSD
> where SOUND_VERSION is less than 4, things will get ugly.
> 
Yes but running packages on an older FreeBSD version than they were
built on isn't really supported (read: won't work, especially if its
an older major branch too like in this case.)  On top of that,
SOUND_VERSION was bumped to 0x040000 for FreeBSD 7.0 already (the
commit is from 2006), and FreeBSD 6 is nothing more than a legacy
branch now that I think few ppl still use (or at least for things
like qemu), the last release was FreeBSD 8.0...

> In the new code i mentioned one can opt for not considering POLICY
> failure a hard error and proceed with a SETFRAGMENT path, which is
> what i would prefer to see on FreeBSD.
> 
 Anyway, you are the qemu audio maintainer, so if thats really what
you prefer, fine with me. :)

 Cheers,
	Juergen

Patch

--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -289,8 +289,25 @@  static int oss_open (int in, struct oss_
 
 #ifdef USE_DSP_POLICY
     if (ioctl (fd, OSS_GETVERSION, &version)) {
+#ifdef __FreeBSD__
+        /*
+	 * Looks like atm (20100109) FreeBSD knows OSS_GETVERSION
+	 * since 7.x, but currently only on the mixer device (or in
+	 * the Linuxolator), and in the native version that part of
+	 * the code is in fact never reached so the ioctl fails anyway.
+	 * But since it just (attempts to) return SOUND_VERSION as
+	 * defined in <sys/soundcard.h> we can fall back to getting it
+	 * from there.  (On FreeBSD, /usr/include is part of the base
+	 * system which is assumed to be always in sync with the kernel.)
+	 * XXX What if the user runs the original OSS from ports instead
+	 * of FreeBSD's own code?  Well I hope we can assume the ioctl
+	 * works there... :)
+	 */
+        version = SOUND_VERSION;
+#else
         oss_logerr2 (errno, typ, "Failed to get OSS version\n");
         version = 0;
+#endif
     }
 
     if (conf.debug) {