Patchwork Fix build if OSS_GETVERSION is not defined

login
register
mail settings
Submitter Juergen Lock
Date Jan. 7, 2010, 10:23 p.m.
Message ID <20100107222343.GA90709@triton8.kn-bremen.de>
Download mbox | patch
Permalink /patch/42483/
State New
Headers show

Comments

Juergen Lock - Jan. 7, 2010, 10:23 p.m.
In this case it was missing on FreeBSD <= 6.x  (Which also doesn't have
SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.)

 Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
malc - Jan. 8, 2010, 8:27 a.m.
On Thu, 7 Jan 2010, Juergen Lock wrote:

> In this case it was missing on FreeBSD <= 6.x  (Which also doesn't have
> SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.)

I've commited slightly different fix for the issue, thanks.
Juergen Lock - Jan. 8, 2010, 7:46 p.m.
On Fri, Jan 08, 2010 at 11:27:13AM +0300, malc wrote:
> On Thu, 7 Jan 2010, Juergen Lock wrote:
> 
> > In this case it was missing on FreeBSD <= 6.x  (Which also doesn't have
> > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.)
> 
> I've commited slightly different fix for the issue, thanks.
> 
Hmm looking at the last hunk of the commit,

>[...]
>@@ -289,9 +292,17 @@ static int oss_open (int in, struct oss_params *req,
>     if (conf.debug) {
>         dolog ("OSS version = %#x\n", version);
>     }
>+#endif
> 
> #ifdef SNDCTL_DSP_POLICY
>-    if (conf.policy >= 0 && version >= 0x040000) {
>+    if (conf.policy >= 0
>+#ifdef OSS_GETVERSION
>+        && version >= 0x040000
>+#else
>+        0

 ...these last two lines (#else and 0) probably should go, I dont think
the compiler likes whitespace between digits. :)

>+#endif
>+        )
>+    {
>         int policy = conf.policy;
>         if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) {
>             oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n",
>-- 
>1.6.6

 And also I forgot to say this is stable-0.12 material too.

 And thanx for committing!
	Juergen
malc - Jan. 8, 2010, 9:33 p.m.
On Fri, 8 Jan 2010, Juergen Lock wrote:

> On Fri, Jan 08, 2010 at 11:27:13AM +0300, malc wrote:
> > On Thu, 7 Jan 2010, Juergen Lock wrote:
> > 
> > > In this case it was missing on FreeBSD <= 6.x  (Which also doesn't have
> > > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.)
> > 
> > I've commited slightly different fix for the issue, thanks.
> > 
> Hmm looking at the last hunk of the commit,
> 
> >[...]
> >@@ -289,9 +292,17 @@ static int oss_open (int in, struct oss_params *req,
> >     if (conf.debug) {
> >         dolog ("OSS version = %#x\n", version);
> >     }
> >+#endif
> > 
> > #ifdef SNDCTL_DSP_POLICY
> >-    if (conf.policy >= 0 && version >= 0x040000) {
> >+    if (conf.policy >= 0
> >+#ifdef OSS_GETVERSION
> >+        && version >= 0x040000
> >+#else
> >+        0
> 
>  ...these last two lines (#else and 0) probably should go, I dont think
> the compiler likes whitespace between digits. :)

Uh, yeah, my bad, sorry, hopefuly fixed now..

> 
> >+#endif
> >+        )
> >+    {
> >         int policy = conf.policy;
> >         if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) {
> >             oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n",
> >-- 
> >1.6.6
> 
>  And also I forgot to say this is stable-0.12 material too.

You really should talk to the people who know what that means :)

>  And thanx for committing!
> 	Juergen
>
Juergen Lock - Jan. 9, 2010, 1:45 p.m.
On Sat, Jan 09, 2010 at 12:33:44AM +0300, malc wrote:
> On Fri, 8 Jan 2010, Juergen Lock wrote:
> 
> > On Fri, Jan 08, 2010 at 11:27:13AM +0300, malc wrote:
> > > On Thu, 7 Jan 2010, Juergen Lock wrote:
> > > 
> > > > In this case it was missing on FreeBSD <= 6.x  (Which also doesn't have
> > > > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.)
> > > 
> > > I've commited slightly different fix for the issue, thanks.
> > > 
> > Hmm looking at the last hunk of the commit,
> > 
> > >[...]
> > >@@ -289,9 +292,17 @@ static int oss_open (int in, struct oss_params *req,
> > >     if (conf.debug) {
> > >         dolog ("OSS version = %#x\n", version);
> > >     }
> > >+#endif
> > > 
> > > #ifdef SNDCTL_DSP_POLICY
> > >-    if (conf.policy >= 0 && version >= 0x040000) {
> > >+    if (conf.policy >= 0
> > >+#ifdef OSS_GETVERSION
> > >+        && version >= 0x040000
> > >+#else
> > >+        0
> > 
> >  ...these last two lines (#else and 0) probably should go, I dont think
> > the compiler likes whitespace between digits. :)
> 
> Uh, yeah, my bad, sorry, hopefuly fixed now..
> 
Yeah looking better now...  Thanx!

> > 
> > >+#endif
> > >+        )
> > >+    {
> > >         int policy = conf.policy;
> > >         if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) {
> > >             oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n",
> > >-- 
> > >1.6.6
> > 
> >  And also I forgot to say this is stable-0.12 material too.
> 
> You really should talk to the people who know what that means :)

 Oh sorry I should have guessed not all committers do merges to stable
branches...  Anthony? :)

 Greets,
	Juergen
Andreas Färber - Jan. 9, 2010, 2:03 p.m.
Am 09.01.2010 um 14:45 schrieb Juergen Lock:

> On Sat, Jan 09, 2010 at 12:33:44AM +0300, malc wrote:
>> On Fri, 8 Jan 2010, Juergen Lock wrote:
>>
>>> And also I forgot to say this is stable-0.12 material too.
>>
>> You really should talk to the people who know what that means :)
>
> Oh sorry I should have guessed not all committers do merges to stable
> branches...  Anthony? :)

In this case you should probably resubmit one accumulated top-level  
patch marked [PATCH][For stable-0.12] or similar (instead of having  
two commits cherry-picked).

Andreas
Juergen Lock - Jan. 9, 2010, 9:08 p.m.
On Sat, Jan 09, 2010 at 03:03:43PM +0100, Andreas Färber wrote:
> 
> Am 09.01.2010 um 14:45 schrieb Juergen Lock:
> 
> > On Sat, Jan 09, 2010 at 12:33:44AM +0300, malc wrote:
> >> On Fri, 8 Jan 2010, Juergen Lock wrote:
> >>
> >>> And also I forgot to say this is stable-0.12 material too.
> >>
> >> You really should talk to the people who know what that means :)
> >
> > Oh sorry I should have guessed not all committers do merges to stable
> > branches...  Anthony? :)
> 
> In this case you should probably resubmit one accumulated top-level  
> patch marked [PATCH][For stable-0.12] or similar (instead of having  
> two commits cherry-picked).

Ok I can do that, but first I have another patch to the same code,
turns out the ioctl doesn't actually work yet on FreeBSD even if it
is defined...

 Cheers,
	Juergen

Patch

--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -240,7 +240,7 @@  static int oss_open (int in, struct oss_
                      struct oss_params *obt, int *pfd)
 {
     int fd;
-    int version;
+    int version = 0;
     int oflags = conf.exclusive ? O_EXCL : 0;
     audio_buf_info abinfo;
     int fmt, freq, nchannels;
@@ -281,10 +281,12 @@  static int oss_open (int in, struct oss_
         goto err;
     }
 
+#ifdef OSS_GETVERSION
     if (ioctl (fd, OSS_GETVERSION, &version)) {
         oss_logerr2 (errno, typ, "Failed to get OSS version\n");
         version = 0;
     }
+#endif
 
     if (conf.debug) {
         dolog ("OSS version = %#x\n", version);