NetBSD Problem Report #50276

From www@NetBSD.org  Thu Sep 24 10:35:20 2015
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 6B3F6A66A7
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 24 Sep 2015 10:35:20 +0000 (UTC)
Message-Id: <20150924103519.3AB84A66C4@mollari.NetBSD.org>
Date: Thu, 24 Sep 2015 10:35:19 +0000 (UTC)
From: rmh@gnu.org
Reply-To: rmh@gnu.org
To: gnats-bugs@NetBSD.org
Subject: [PATCH] Portability fixes for ossaudio.c
X-Send-Pr-Version: www-1.0

>Number:         50276
>Category:       lib
>Synopsis:       [PATCH] Portability fixes for ossaudio.c
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Thu Sep 24 10:40:00 +0000 2015
>Last-Modified:  Sat Sep 26 20:55:00 +0000 2015
>Originator:     Robert Millan
>Release:        HEAD
>Organization:
>Environment:
>Description:
Thanks to Rump, it is now possible to use NetBSD audio drivers on other operating systems. It follows that libossaudio can be used (and is in fact very useful) on those operating systems too.

In order to preserve ABI compatibility with applications that were linked to use OSS using the native <sys/soundcard.h> of those systems, it becomes really useful to build libossaudio using the system-wide <sys/soundcard.h> too.

On GNU/Linux & GNU/Hurd systems this required a few trivial portability adjustments as some of the features are not available there.

As you can see, the patch is very non-intrusive (it just checks for features before attempting to use them).
>How-To-Repeat:

>Fix:

>Audit-Trail:
From: Robert Millan <rmh@gnu.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 12:41:56 +0200

 This is a multi-part message in MIME format.
 --------------010508030407060203010008
 Content-Type: text/plain; charset=windows-1252; format=flowed
 Content-Transfer-Encoding: 7bit


 Here's the patch.

 El 24/09/15 a les 12:40, gnats-admin@netbsd.org ha escrit:
 > Thank you very much for your problem report.
 > It has the internal identification `lib/50276'.
 > The individual assigned to look at your
 > report is: lib-bug-people.
 >
 >> Category:       lib
 >> Responsible:    lib-bug-people
 >> Synopsis:       [PATCH] Portability fixes for ossaudio.c
 >> Arrival-Date:   Thu Sep 24 10:40:00 +0000 2015
 >

 -- 
 Robert Millan

 --------------010508030407060203010008
 Content-Type: text/x-patch;
  name="ossaudio.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="ossaudio.diff"

 diff --git a/ossaudio.c b/ossaudio.c
 index 3a7773b..f7ede59 100644
 --- a/ossaudio.c
 +++ b/ossaudio.c
 @@ -96,7 +101,9 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  	struct audio_buf_info bufinfo;
  	struct count_info cntinfo;
  	struct audio_encoding tmpenc;
 +#ifdef SNDCTL_SYSINFO
  	struct oss_sysinfo tmpsysinfo;
 +#endif
  	struct oss_audioinfo *tmpaudioinfo;
  	audio_device_t tmpaudiodev;
  	struct stat tmpstat;
 @@ -204,30 +211,38 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  			tmpinfo.play.encoding =
  			tmpinfo.record.encoding = AUDIO_ENCODING_ULINEAR_BE;
  			break;
 +#ifdef AFMT_S24_LE
  		case AFMT_S24_LE:
  			tmpinfo.play.precision =
  			tmpinfo.record.precision = 24;
  			tmpinfo.play.encoding =
  			tmpinfo.record.encoding = AUDIO_ENCODING_SLINEAR_LE;
  			break;
 +#endif
 +#ifdef AFMT_S24_BE
  		case AFMT_S24_BE:
  			tmpinfo.play.precision =
  			tmpinfo.record.precision = 24;
  			tmpinfo.play.encoding =
  			tmpinfo.record.encoding = AUDIO_ENCODING_SLINEAR_BE;
  			break;
 +#endif
 +#ifdef AFMT_S32_LE
  		case AFMT_S32_LE:
  			tmpinfo.play.precision =
  			tmpinfo.record.precision = 32;
  			tmpinfo.play.encoding =
  			tmpinfo.record.encoding = AUDIO_ENCODING_SLINEAR_LE;
  			break;
 +#endif
 +#ifdef AFMT_S32_BE
  		case AFMT_S32_BE:
  			tmpinfo.play.precision =
  			tmpinfo.record.precision = 32;
  			tmpinfo.play.encoding =
  			tmpinfo.record.encoding = AUDIO_ENCODING_SLINEAR_BE;
  			break;
 +#endif
  		case AFMT_AC3:
  			tmpinfo.play.precision =
  			tmpinfo.record.precision = 16;
 @@ -251,21 +266,33 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  			idat = AFMT_A_LAW;
  			break;
  		case AUDIO_ENCODING_SLINEAR_LE:
 +#ifdef AFMT_S32_LE
  			if (tmpinfo.play.precision == 32)
  				idat = AFMT_S32_LE;
 -			else if (tmpinfo.play.precision == 24)
 +			else
 +#endif
 +#ifdef AFMT_S24_LE
 +			if (tmpinfo.play.precision == 24)
  				idat = AFMT_S24_LE;
 -			else if (tmpinfo.play.precision == 16)
 +			else
 +#endif
 +			if (tmpinfo.play.precision == 16)
  				idat = AFMT_S16_LE;
  			else
  				idat = AFMT_S8;
  			break;
  		case AUDIO_ENCODING_SLINEAR_BE:
 +#ifdef AFMT_S32_BE
  			if (tmpinfo.play.precision == 32)
  				idat = AFMT_S32_BE;
 -			else if (tmpinfo.play.precision == 24)
 +			else
 +#endif
 +#ifdef AFMT_S24_BE
 +			if (tmpinfo.play.precision == 24)
  				idat = AFMT_S24_BE;
 -			else if (tmpinfo.play.precision == 16)
 +			else
 +#endif
 +			if (tmpinfo.play.precision == 16)
  				idat = AFMT_S16_BE;
  			else
  				idat = AFMT_S8;
 @@ -359,21 +386,33 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  				idat |= AFMT_S8;
  				break;
  			case AUDIO_ENCODING_SLINEAR_LE:
 +#ifdef AFMT_S32_LE
  				if (tmpenc.precision == 32)
  					idat |= AFMT_S32_LE;
 -				else if (tmpenc.precision == 24)
 +				else
 +#endif
 +#ifdef AFMT_S24_LE
 +				if (tmpenc.precision == 24)
  					idat |= AFMT_S24_LE;
 -				else if (tmpenc.precision == 16)
 +				else
 +#endif
 +				if (tmpenc.precision == 16)
  					idat |= AFMT_S16_LE;
  				else
  					idat |= AFMT_S8;
  				break;
  			case AUDIO_ENCODING_SLINEAR_BE:
 +#ifdef AFMT_S32_BE
  				if (tmpenc.precision == 32)
  					idat |= AFMT_S32_BE;
 -				else if (tmpenc.precision == 24)
 +				else
 +#endif
 +#ifdef AFMT_S24_BE
 +				if (tmpenc.precision == 24)
  					idat |= AFMT_S24_BE;
 -				else if (tmpenc.precision == 16)
 +				else
 +#endif
 +				if (tmpenc.precision == 16)
  					idat |= AFMT_S16_BE;
  				else
  					idat |= AFMT_S8;
 @@ -496,7 +535,8 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  		cntinfo.ptr = tmpoffs.offset;
  		*(struct count_info *)argp = cntinfo;
  		break;
 -	case SNDCTL_SYSINFO:
 +#ifdef SNDCTL_SYSINFO
 + 	case SNDCTL_SYSINFO:
  		strncpy(tmpsysinfo.product, "OSS/NetBSD", 31);
  		tmpsysinfo.product[31] = 0; 
  		strncpy(tmpsysinfo.version, version, 31); 
 @@ -516,8 +556,14 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  		memset(tmpsysinfo.openedmidi, 0, 8);
  		*(struct oss_sysinfo *)argp = tmpsysinfo;
  		break;
 +#endif
 +#if defined(SNDCTL_ENGINEINFO) || defined(SNDCTL_AUDIOINFO)
 +#ifdef SNDCTL_ENGINEINFO
  	case SNDCTL_ENGINEINFO:
 +#endif
 +#ifdef SNDCTL_AUDIOINFO
  	case SNDCTL_AUDIOINFO:
 +#endif
  		devno = 0;
  		tmpaudioinfo = (struct oss_audioinfo*)argp;
  		if (tmpaudioinfo == NULL)
 @@ -581,12 +627,16 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  		tmpaudioinfo->next_rec_engine = 0;
  		argp = tmpaudioinfo;
  		break;
 +#endif
 +#ifdef SNDCTL_DSP_GETPLAYVOL
  	case SNDCTL_DSP_GETPLAYVOL:
  		retval = ioctl(fd, AUDIO_GETBUFINFO, &tmpinfo);
  		if (retval < 0)
  			return retval;
  		*(uint *)argp = tmpinfo.play.gain;
  		break;
 +#endif
 +#ifdef SNDCTL_DSP_SETPLAYVOL
  	case SNDCTL_DSP_SETPLAYVOL:
  		retval = ioctl(fd, AUDIO_GETBUFINFO, &tmpinfo);
  		if (retval < 0)
 @@ -599,12 +649,16 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  		if (retval < 0)
  			return retval;
  		break;
 +#endif
 +#ifdef SNDCTL_DSP_GETRECVOL
  	case SNDCTL_DSP_GETRECVOL:
  		retval = ioctl(fd, AUDIO_GETBUFINFO, &tmpinfo);
  		if (retval < 0)
  			return retval;
  		*(uint *)argp = tmpinfo.record.gain;
  		break;
 +#endif
 +#ifdef SNDCTL_DSP_SETRECVOL
  	case SNDCTL_DSP_SETRECVOL:
  		retval = ioctl(fd, AUDIO_GETBUFINFO, &tmpinfo);
  		if (retval < 0)
 @@ -617,9 +671,15 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  		if (retval < 0)
  			return retval;
  		break;
 +#endif
 +#ifdef SNDCTL_DSP_SKIP
  	case SNDCTL_DSP_SKIP:
 +		return EINVAL;
 +#endif
 +#ifdef SNDCTL_DSP_SILENCE
  	case SNDCTL_DSP_SILENCE:
  		return EINVAL;
 +#endif
  	case SNDCTL_DSP_SETDUPLEX:
  		idat = 1;
  		retval = ioctl(fd, AUDIO_SETFD, &idat);
 @@ -638,8 +698,12 @@ audio_ioctl(int fd, unsigned long com, void *argp)
  		 * implementing it as a NOP is ok
  		 */     
  		break;
 +#ifdef SNDCTL_DSP_MAPINBUF
  	case SNDCTL_DSP_MAPINBUF:
 +#endif
 +#ifdef SNDCTL_DSP_MAPOUTBUF
  	case SNDCTL_DSP_MAPOUTBUF:
 +#endif
  	case SNDCTL_DSP_SETSYNCRO:
  		errno = EINVAL;
  		return -1; /* XXX unimplemented */
 @@ -904,7 +968,9 @@ mixer_ioctl(int fd, unsigned long com, void *argp)
  		idat = di->caps;
  		break;
  	case SOUND_MIXER_WRITE_RECSRC:
 +#ifdef SOUND_MIXER_WRITE_R_RECSRC
  	case SOUND_MIXER_WRITE_R_RECSRC:
 +#endif
  		if (di->source == -1)
  			return EINVAL;
  		mc.dev = di->source;
 @@ -932,6 +998,9 @@ mixer_ioctl(int fd, unsigned long com, void *argp)
  		}
  		return ioctl(fd, AUDIO_MIXER_WRITE, &mc);
  	default:
 +#ifndef SOUND_MIXER_FIRST
 +#define SOUND_MIXER_FIRST 0
 +#endif
  		if (MIXER_READ(SOUND_MIXER_FIRST) <= com &&
  		    com < MIXER_READ(SOUND_MIXER_NRDEVICES)) {
  			n = GET_DEV(com);
 @@ -956,8 +1025,11 @@ mixer_ioctl(int fd, unsigned long com, void *argp)
  			}
  			idat = TO_OSSVOL(l) | (TO_OSSVOL(r) << 8);
  			break;
 -		} else if ((MIXER_WRITE_R(SOUND_MIXER_FIRST) <= com &&
 +		} else if (
 +#ifdef MIXER_WRITE_R
 +			   (MIXER_WRITE_R(SOUND_MIXER_FIRST) <= com &&
  			   com < MIXER_WRITE_R(SOUND_MIXER_NRDEVICES)) ||
 +#endif
  			   (MIXER_WRITE(SOUND_MIXER_FIRST) <= com &&
  			   com < MIXER_WRITE(SOUND_MIXER_NRDEVICES))) {
  			n = GET_DEV(com);

 --------------010508030407060203010008--

From: Antti Kantee <pooka@iki.fi>
To: rmh@gnu.org, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 14:02:17 +0000

 On 24/09/15 10:43, Robert Millan wrote:
 >
 > FYI:
 >
 > https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50276

 If Hurd soundcard.h is missing those defines, doesn't it mean that some 
 ossaudio programs will fail to compile on Hurd?  Why isn't adding the 
 missing defines to the Hurd soundcard.h possible?  The ABI used by old 
 binaries will not change if you add new definitions, so I'm not sure I 
 understand your motivation for patching NetBSD sources here.

From: Antti Kantee <pooka@iki.fi>
To: rmh@gnu.org, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 14:26:55 +0000

 On 24/09/15 14:18, Antti Kantee wrote:
 > Also, if you want to use just ossaudio on Hurd, wouldn't it be better to
 > use the in-(rump-)kernel ossaudio compat translator (sys/compat/ossaudio)?

 Then again, I don't know if Hurd soundcard.h matches the one that 
 sys/compat/ossaudio expects.  I guess the compat module could be taught 
 about the Hurd ioctl's, though.

 Anyway, many many ways to go about this ...

From: Antti Kantee <pooka@iki.fi>
To: rmh@gnu.org, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 14:18:31 +0000

 On 24/09/15 14:02, Antti Kantee wrote:
 > On 24/09/15 10:43, Robert Millan wrote:
 >>
 >> FYI:
 >>
 >> https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50276
 >
 > If Hurd soundcard.h is missing those defines, doesn't it mean that some
 > ossaudio programs will fail to compile on Hurd?  Why isn't adding the
 > missing defines to the Hurd soundcard.h possible?  The ABI used by old
 > binaries will not change if you add new definitions, so I'm not sure I
 > understand your motivation for patching NetBSD sources here.

 Also, if you want to use just ossaudio on Hurd, wouldn't it be better to 
 use the in-(rump-)kernel ossaudio compat translator (sys/compat/ossaudio)?

From: Robert Millan <rmh@gnu.org>
To: Antti Kantee <pooka@iki.fi>, rumpkernel-users@freelists.org
Cc: Bug hurd mailing list <bug-hurd@gnu.org>, gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 20:09:27 +0200

 Hi Antti,

 Adding bug-hurd to CC since some of the issues may concern them.

 El 24/09/15 a les 16:02, Antti Kantee ha escrit:
 > On 24/09/15 10:43, Robert Millan wrote:
 >>
 >> FYI:
 >>
 >> https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50276
 >
 > If Hurd soundcard.h is missing those defines, doesn't it mean that some ossaudio programs will fail to compile on Hurd?

 Note that most of the defines are missing on Linux too (the only exception being
 SNDCTL_DSP_MAPINBUF and SNDCTL_DSP_MAPOUTBUF).

 Aside from SNDCTL_DSP_MAPINBUF/SNDCTL_DSP_MAPOUTBUF potentially being an issue
 with Hurd version of <sys/soundcard.h>, the problem is not with applications
 but with compiling ossaudio.c itself.

 > Why isn't adding the missing defines to the Hurd soundcard.h possible?

 Not just possible but also desirable IMHO.

 My point however isn't about running libossaudio in a specific OS, but rather
 that since libossaudio is now potentially usable on just about every system that
 could run Rump, I think it makes sense to make libossaudio more portable in general.

 However if you disagree about that goal, it's no big deal. Users of this stack
 can keep using it and carry the patches if needed.

 > The ABI used by old binaries will not change if you add new definitions, so I'm not sure I understand your motivation for patching NetBSD sources here.

 Please also note that sys/soundcard.h is a vendor supplied header. Although on
 Hurd I expect the developers would agree to improve sys/soundcard.h, in some
 cases it's next to impossible.

 Consider for example that on Linux this header is provided by the kernel, and:
    - their attitude towards OSS (which they consider deprecated in favour of ALSA)
    - their attitude towards user-space device driving on their platform (our recent
      discussion about MSI support in UIO comes to mind here).

 -- 
 Robert Millan

From: Robert Millan <rmh@gnu.org>
To: pooka@iki.fi, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 20:15:37 +0200

 El 24/09/15 a les 16:18, Antti Kantee ha escrit:
 > On 24/09/15 14:02, Antti Kantee wrote:
 >> On 24/09/15 10:43, Robert Millan wrote:
 >>>
 >>> FYI:
 >>>
 >>> https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=50276
 >>
 >> If Hurd soundcard.h is missing those defines, doesn't it mean that some
 >> ossaudio programs will fail to compile on Hurd?  Why isn't adding the
 >> missing defines to the Hurd soundcard.h possible?  The ABI used by old
 >> binaries will not change if you add new definitions, so I'm not sure I
 >> understand your motivation for patching NetBSD sources here.
 >
 > Also, if you want to use just ossaudio on Hurd, wouldn't it be better to use the in-(rump-)kernel ossaudio compat translator (sys/compat/ossaudio)?

 There's a big problem with using sys/compat/ossaudio on non-NetBSD: since
 it is built in the kernel tree, it includes the NetBSD version of <sys/soundcard.h>
 and also other headers like <sys/audioio.h> and <sys/ioccom.h>.

 As a result it can't be ABI-compatible with applications that were built
 using the system-wide version of <sys/soundcard.h>.

 Note that rebuilding world is not supported with binary-based distributions, unlike
 systems of BSD tradition where it's usually a trivial task.

 -- 
 Robert Millan

From: Antti Kantee <pooka@iki.fi>
To: rmh@gnu.org, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 18:21:51 +0000

 On 24/09/15 18:15, Robert Millan wrote:
 >> Also, if you want to use just ossaudio on Hurd, wouldn't it be better
 >> to use the in-(rump-)kernel ossaudio compat translator
 >> (sys/compat/ossaudio)?
 >
 > There's a big problem with using sys/compat/ossaudio on non-NetBSD: since
 > it is built in the kernel tree, it includes the NetBSD version of
 > <sys/soundcard.h>
 > and also other headers like <sys/audioio.h> and <sys/ioccom.h>.

 No, it does *not* include NetBSD <sys/soundcard.h>.  The entire point of 
 the module is to not use it(!)

 > As a result it can't be ABI-compatible with applications that were built
 > using the system-wide version of <sys/soundcard.h>.
 >
 > Note that rebuilding world is not supported with binary-based
 > distributions, unlike
 > systems of BSD tradition where it's usually a trivial task.

From: Samuel Thibault <samuel.thibault@gnu.org>
To: Robert Millan <rmh@gnu.org>
Cc: Antti Kantee <pooka@iki.fi>, rumpkernel-users@freelists.org,
	gnats-bugs@NetBSD.org, Bug hurd mailing list <bug-hurd@gnu.org>
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 20:18:37 +0200

 Hello,

 Robert Millan, le Thu 24 Sep 2015 20:09:27 +0200, a écrit :
 > >Why isn't adding the missing defines to the Hurd soundcard.h possible?
 > 
 > Not just possible but also desirable IMHO.

 It may still be impossible at the moment actually.  The
 SNDCTL_DSP_MAPINBUF and SNDCTL_DSP_MAPOUTBUF ioctls are precisely
 examples: they pass a struct which contains the address of a buffer.
 This can not be expressed with the current handling of ioctls in
 GNU/Hurd: the sound server can't read/write the client process memory.

 Samuel

From: Robert Millan <rmh@gnu.org>
To: Antti Kantee <pooka@iki.fi>, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 21:50:28 +0200

 El 24/09/15 a les 20:21, Antti Kantee ha escrit:
 > On 24/09/15 18:15, Robert Millan wrote:
 >>> Also, if you want to use just ossaudio on Hurd, wouldn't it be better
 >>> to use the in-(rump-)kernel ossaudio compat translator
 >>> (sys/compat/ossaudio)?
 >>
 >> There's a big problem with using sys/compat/ossaudio on non-NetBSD: since
 >> it is built in the kernel tree, it includes the NetBSD version of
 >> <sys/soundcard.h>
 >> and also other headers like <sys/audioio.h> and <sys/ioccom.h>.
 >
 > No, it does *not* include NetBSD <sys/soundcard.h>.  The entire point of the module is to not use it(!)

 Sorry I used the wrong wording, what I meant to say is that it expects ioctl parameters
 using a different ABI than the one provided by system soundcard.h.

 My understanding is that using compat/ossaudio to support userland OSS applications
 on non-NetBSD systems would require writing an intermediate layer to do the ABI
 translation. And performing a few minor adjustments to libossaudio is much easier
 than writing an ABI conversion layer :-)

 -- 
 Robert Millan

From: Antti Kantee <pooka@iki.fi>
To: rmh@gnu.org, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Thu, 24 Sep 2015 22:10:39 +0000

 On 24/09/15 19:50, Robert Millan wrote:
 > El 24/09/15 a les 20:21, Antti Kantee ha escrit:
 >> On 24/09/15 18:15, Robert Millan wrote:
 >>>> Also, if you want to use just ossaudio on Hurd, wouldn't it be better
 >>>> to use the in-(rump-)kernel ossaudio compat translator
 >>>> (sys/compat/ossaudio)?
 >>>
 >>> There's a big problem with using sys/compat/ossaudio on non-NetBSD:
 >>> since
 >>> it is built in the kernel tree, it includes the NetBSD version of
 >>> <sys/soundcard.h>
 >>> and also other headers like <sys/audioio.h> and <sys/ioccom.h>.
 >>
 >> No, it does *not* include NetBSD <sys/soundcard.h>.  The entire point
 >> of the module is to not use it(!)
 >
 > Sorry I used the wrong wording, what I meant to say is that it expects
 > ioctl parameters
 > using a different ABI than the one provided by system soundcard.h.
 >
 > My understanding is that using compat/ossaudio to support userland OSS
 > applications
 > on non-NetBSD systems would require writing an intermediate layer to do
 > the ABI
 > translation. And performing a few minor adjustments to libossaudio is
 > much easier
 > than writing an ABI conversion layer :-)

 The premise for using libossaudio is that you are calling 
 rump_sys_ioctl() (= ABI minus ioctl params).  libossaudio mangles the 
 ioctl params before the ioctl() call ("explicitly").  compat/ossaudio 
 mangles the params after the call but before passing them 
 ("implicitly").  Apart from where the ioctl parameters are translated 
 there is no [fundamental] difference between libossaudio and 
 compat/ossaudio.

 Let me put it this way: the sys/soundcard.h that libossaudio installs 
 redefines ioctl so that it catches the ioctl calls before they go hit 
 the kernel.  How does that work with a stock soundcard.h from somewhere 
 else?  Is there some indirection in your stack somewhere that I'm not 
 aware of?

 The reason I'm trying to find alternate solutions is that I find 
 manually sprinkling ifdefs into the switch quite ugly and fragile and 
 would rather see a cleaner solution.  Another way to work around that 
 problem is figure out how to use some sort of translator before the 
 switch so that the primary code path doesn't get polluted with ifdefs 
 ... or to find someone who's not bothered by ifdefs to commit your patch :/

From: Robert Millan <rmh@gnu.org>
To: Antti Kantee <pooka@iki.fi>, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Fri, 25 Sep 2015 21:29:23 +0200

 El 25/09/15 a les 00:10, Antti Kantee ha escrit:
 > Let me put it this way: the sys/soundcard.h that libossaudio installs redefines ioctl so that it catches the ioctl calls before they go hit the kernel.  How does that work with a stock soundcard.h from somewhere else?  Is there some indirection in your stack somewhere that I'm not aware of?

 No indirection, it's simpler than that!

 Take for example:

          switch (com) {
          case SNDCTL_DSP_RESET:
                  retval = ioctl(fd, AUDIO_FLUSH, 0);
                  if (retval < 0)
                          return retval;
                  break;

 ossaudio.c is built using system-wide <sys/soundcard.h>. In turn, the ioctl
 constant definitions in <sys/soundcard.h> use the system-wide _IOC macros.

 At the same time, ossaudio.c is built using the NetBSD version of sys/audioio.h,
 which in turn uses _IOC macros by the NetBSD version of sys/ioccom.h [1].

 So in the example above, the "case SNDCTL_DSP_RESET" statement is using
 system-wide ABI (both in the sense of system-wide <sys/soundcard.h> and
 in the sense of system-wide _IOC), whereas the "ioctl(fd, AUDIO_FLUSH, 0)"
 example is using NetBSD ABI (in both senses too).

 [1] Note: both files are modified to avoid namespace collision, see
      https://github.com/robertmillan/rumposs/commit/dd5596f20126e54b723d312ba5ad182747a38b9b

 > The reason I'm trying to find alternate solutions is that I find manually sprinkling ifdefs into the switch quite ugly and fragile and would rather see a cleaner solution.  Another way to work around that problem is figure out how to use some sort of translator before the switch so that the primary code path doesn't get polluted with ifdefs ... or to find someone who's not bothered by ifdefs to commit your patch :/

 Are you concerned with all ifdefs or just with the ones that get inside
 "if () {}" statements? I think the later could possibly be replaced with
 ifndef/define before the switch. Does this look better to you?

 -- 
 Robert Millan

From: Olaf Buddenhagen <olafbuddenhagen@gmx.net>
To: Robert Millan <rmh@gnu.org>, Antti Kantee <pooka@iki.fi>,
	rumpkernel-users@freelists.org, gnats-bugs@NetBSD.org,
	Bug hurd mailing list <bug-hurd@gnu.org>
Cc: 
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Sat, 26 Sep 2015 15:08:15 +0200

 Hi,

 On Thu, Sep 24, 2015 at 08:18:37PM +0200, Samuel Thibault wrote:

 > It may still be impossible at the moment actually.  The
 > SNDCTL_DSP_MAPINBUF and SNDCTL_DSP_MAPOUTBUF ioctls are precisely
 > examples: they pass a struct which contains the address of a buffer.
 > This can not be expressed with the current handling of ioctls in
 > GNU/Hurd: the sound server can't read/write the client process memory.

 AIUI, it is possible by adding explicit stubs for these specific ioctls
 to our glibc. I think this is used both for ioctls with too many
 parameters to encode them with the standard RPC ID mapping scheme, and
 for ioctls with "problematic" parameters like these?

 -antrik-

From: Antti Kantee <pooka@iki.fi>
To: rmh@gnu.org, rumpkernel-users@freelists.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Sat, 26 Sep 2015 14:31:07 +0000

 On 25/09/15 19:29, Robert Millan wrote:
 > El 25/09/15 a les 00:10, Antti Kantee ha escrit:
 >> Let me put it this way: the sys/soundcard.h that libossaudio installs
 >> redefines ioctl so that it catches the ioctl calls before they go hit
 >> the kernel.  How does that work with a stock soundcard.h from
 >> somewhere else?  Is there some indirection in your stack somewhere
 >> that I'm not aware of?
 >
 > No indirection, it's simpler than that!
 >
 > Take for example:
 >
 >          switch (com) {
 >          case SNDCTL_DSP_RESET:
 >                  retval = ioctl(fd, AUDIO_FLUSH, 0);
 >                  if (retval < 0)
 >                          return retval;
 >                  break;
 >
 > ossaudio.c is built using system-wide <sys/soundcard.h>. In turn, the ioctl
 > constant definitions in <sys/soundcard.h> use the system-wide _IOC macros.

 Ah, ossaudio.c is build using Hurd's soundcard.h.  Yea, don't know what 
 I was thinking, but that explains it ...

 > At the same time, ossaudio.c is built using the NetBSD version of
 > sys/audioio.h,
 > which in turn uses _IOC macros by the NetBSD version of sys/ioccom.h [1].

 ... and that explains it even more.

 >> The reason I'm trying to find alternate solutions is that I find
 >> manually sprinkling ifdefs into the switch quite ugly and fragile and
 >> would rather see a cleaner solution.  Another way to work around that
 >> problem is figure out how to use some sort of translator before the
 >> switch so that the primary code path doesn't get polluted with ifdefs
 >> ... or to find someone who's not bothered by ifdefs to commit your
 >> patch :/
 >
 > Are you concerned with all ifdefs or just with the ones that get inside
 > "if () {}" statements? I think the later could possibly be replaced with
 > ifndef/define before the switch. Does this look better to you?

 Or, you could have a #include "hurd_compat.h" or something like that, 
 which defines the missing macros to invalid values which are never used 
 by the caller, e.g.

 #define SNDCTL_FOO -1
 #define SNDCTL_BAR -2

 etc...

 That way the compat is compartmentalized and generally nicer.

 For the AFMT values, can't you just add those to Hurd?  Aren't they just 
 arbitrary constants?

From: Samuel Thibault <samuel.thibault@gnu.org>
To: Robert Millan <rmh@gnu.org>, Antti Kantee <pooka@iki.fi>,
	rumpkernel-users@freelists.org, gnats-bugs@NetBSD.org,
	Bug hurd mailing list <bug-hurd@gnu.org>
Cc: 
Subject: Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c
Date: Sat, 26 Sep 2015 22:50:42 +0200

 Olaf Buddenhagen, le Sat 26 Sep 2015 15:08:15 +0200, a écrit :
 > On Thu, Sep 24, 2015 at 08:18:37PM +0200, Samuel Thibault wrote:
 > 
 > > It may still be impossible at the moment actually.  The
 > > SNDCTL_DSP_MAPINBUF and SNDCTL_DSP_MAPOUTBUF ioctls are precisely
 > > examples: they pass a struct which contains the address of a buffer.
 > > This can not be expressed with the current handling of ioctls in
 > > GNU/Hurd: the sound server can't read/write the client process memory.
 > 
 > AIUI, it is possible by adding explicit stubs for these specific ioctls
 > to our glibc. I think this is used both for ioctls with too many
 > parameters to encode them with the standard RPC ID mapping scheme, and
 > for ioctls with "problematic" parameters like these?

 Perhaps.  I don't know any such example.

 Samuel

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.