Re: Fwd: Re: lib/50276: [PATCH] Portability fixes for ossaudio.c

[Antti Kantee <pooka%iki.fi@localhost>]

The following reply was made to PR lib/50276; it has been noted by GNATS.

From: Antti Kantee <pooka%iki.fi@localhost>
To: rmh%gnu.org@localhost, rumpkernel-users%freelists.org@localhost
Cc: gnats-bugs%NetBSD.org@localhost
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?