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
(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.
Home