uaudio(4): fix: free params on close()

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

uaudio(4): fix: free params on close()

Alexandre Ratchov-2
audio play and/or record parameters are set every time the audio
device is opened. In the uaudio(4) driver the parameter set (ie the
as_info structure) is tagged as busy until the next time parameters
are changed. But some times the device may be closed before the
parameters set is freed; in this case it can no more be used again.

the attached patch fixes this, by making the uaudio_close() release
the as_info structures in use.

to test the patch, open and immediately close the uaudio device,
for instance:

        cat /dev/null >/dev/audio

this will allocate the play.rate=8000 setting, but will not free it.
Then try to set the play.rate parameter to its current value:

        audioctl play.rate=8000

without the patch, audioctl(1) will fail because the parameter is
still tagged as busy.

--
Alexandre
Index: uaudio.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.35
diff -u -r1.35 uaudio.c
--- uaudio.c 21 Apr 2006 15:47:27 -0000 1.35
+++ uaudio.c 27 Apr 2006 21:12:51 -0000
@@ -2096,6 +2096,11 @@
 Static void
 uaudio_close(void *addr)
 {
+ struct uaudio_softc *sc = addr;
+ if (sc->sc_playchan.altidx != -1)
+ sc->sc_alts[sc->sc_playchan.altidx].sc_busy = 0;
+ if (sc->sc_recchan.altidx != -1)
+ sc->sc_alts[sc->sc_recchan.altidx].sc_busy = 0;
 }
 
 Static int

Reply | Threaded
Open this post in threaded view
|

Re: uaudio(4): fix: free params on close()

Michael Shalayeff-2
On Sun, Apr 30, 2006 at 06:52:44PM +0200, Alexandre Ratchov wrote:

> audio play and/or record parameters are set every time the audio
> device is opened. In the uaudio(4) driver the parameter set (ie the
> as_info structure) is tagged as busy until the next time parameters
> are changed. But some times the device may be closed before the
> parameters set is freed; in this case it can no more be used again.
>
> the attached patch fixes this, by making the uaudio_close() release
> the as_info structures in use.
>
> to test the patch, open and immediately close the uaudio device,
> for instance:
>
> cat /dev/null >/dev/audio
>
> this will allocate the play.rate=8000 setting, but will not free it.
> Then try to set the play.rate parameter to its current value:
>
> audioctl play.rate=8000
>
> without the patch, audioctl(1) will fail because the parameter is
> still tagged as busy.

would not calling uaudio_chan_close() be the right thing then?

> Index: uaudio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
> retrieving revision 1.35
> diff -u -r1.35 uaudio.c
> --- uaudio.c 21 Apr 2006 15:47:27 -0000 1.35
> +++ uaudio.c 27 Apr 2006 21:12:51 -0000
> @@ -2096,6 +2096,11 @@
>  Static void
>  uaudio_close(void *addr)
>  {
> + struct uaudio_softc *sc = addr;
> + if (sc->sc_playchan.altidx != -1)
> + sc->sc_alts[sc->sc_playchan.altidx].sc_busy = 0;
> + if (sc->sc_recchan.altidx != -1)
> + sc->sc_alts[sc->sc_recchan.altidx].sc_busy = 0;
>  }
>  
>  Static int
>

--
    paranoic mickey       (my employers have changed but, the name has remained)

Reply | Threaded
Open this post in threaded view
|

Re: uaudio(4): fix: free params on close()

Alexandre Ratchov-2
On Mon, May 01, 2006 at 02:59:03PM +0200, mickey wrote:

> On Sun, Apr 30, 2006 at 06:52:44PM +0200, Alexandre Ratchov wrote:
> > audio play and/or record parameters are set every time the audio
> > device is opened. In the uaudio(4) driver the parameter set (ie the
> > as_info structure) is tagged as busy until the next time parameters
> > are changed. But some times the device may be closed before the
> > parameters set is freed; in this case it can no more be used again.
> >
> > the attached patch fixes this, by making the uaudio_close() release
> > the as_info structures in use.
> >
> > to test the patch, open and immediately close the uaudio device,
> > for instance:
> >
> > cat /dev/null >/dev/audio
> >
> > this will allocate the play.rate=8000 setting, but will not free it.
> > Then try to set the play.rate parameter to its current value:
> >
> > audioctl play.rate=8000
> >
> > without the patch, audioctl(1) will fail because the parameter is
> > still tagged as busy.
>
> would not calling uaudio_chan_close() be the right thing then?
>

uaudio_chan_close() should be called in order to close a chan opened
with uaudio_chan_open(). Chans are opened when input or output is
triggered.

However, parameters set (struct as_info) may be tagged as busy
without any channel being opened (so no chan to close). For instance,
this may happen if the device is opened and then closed (no read() or
write() is called).

--
Alexandre

Reply | Threaded
Open this post in threaded view
|

Re: uaudio(4): fix: free params on close()

Michael Shalayeff-2
On Mon, May 01, 2006 at 05:05:16PM +0200, Alexandre Ratchov wrote:

> On Mon, May 01, 2006 at 02:59:03PM +0200, mickey wrote:
> > On Sun, Apr 30, 2006 at 06:52:44PM +0200, Alexandre Ratchov wrote:
> > > audio play and/or record parameters are set every time the audio
> > > device is opened. In the uaudio(4) driver the parameter set (ie the
> > > as_info structure) is tagged as busy until the next time parameters
> > > are changed. But some times the device may be closed before the
> > > parameters set is freed; in this case it can no more be used again.
> > >
> > > the attached patch fixes this, by making the uaudio_close() release
> > > the as_info structures in use.
> > >
> > > to test the patch, open and immediately close the uaudio device,
> > > for instance:
> > >
> > > cat /dev/null >/dev/audio
> > >
> > > this will allocate the play.rate=8000 setting, but will not free it.
> > > Then try to set the play.rate parameter to its current value:
> > >
> > > audioctl play.rate=8000
> > >
> > > without the patch, audioctl(1) will fail because the parameter is
> > > still tagged as busy.
> >
> > would not calling uaudio_chan_close() be the right thing then?
> >
>
> uaudio_chan_close() should be called in order to close a chan opened
> with uaudio_chan_open(). Chans are opened when input or output is
> triggered.
>
> However, parameters set (struct as_info) may be tagged as busy
> without any channel being opened (so no chan to close). For instance,
> this may happen if the device is opened and then closed (no read() or
> write() is called).

uaudio_chan_close() deals olrite w/ deallocating all the resources
if needed as well as resetting the busy flag.
it is more consistant than just resetting the flag w/o checking
that all the resources are not allocated as well.
so how about this diff?

cu

--
    paranoic mickey       (my employers have changed but, the name has remained)

Index: uaudio.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.35
@@ -2096,6 +2099,12 @@
 Static void
 uaudio_close(void *addr)
 {
+ struct uaudio_softc *sc = addr;
+
+ if (sc->sc_playchan.altidx != -1)
+ uaudio_chan_close(sc, &sc->sc_playchan);
+ if (sc->sc_recchan.altidx != -1)
+ uaudio_chan_close(sc, &sc->sc_recchan);
 }
 
 Static int

Reply | Threaded
Open this post in threaded view
|

Re: uaudio(4): fix: free params on close()

Alexandre Ratchov-2
On Wed, May 03, 2006 at 02:38:40PM +0200, mickey wrote:

> On Mon, May 01, 2006 at 05:05:16PM +0200, Alexandre Ratchov wrote:
> > On Mon, May 01, 2006 at 02:59:03PM +0200, mickey wrote:
> > > On Sun, Apr 30, 2006 at 06:52:44PM +0200, Alexandre Ratchov wrote:
> > > > audio play and/or record parameters are set every time the audio
> > > > device is opened. In the uaudio(4) driver the parameter set (ie the
> > > > as_info structure) is tagged as busy until the next time parameters
> > > > are changed. But some times the device may be closed before the
> > > > parameters set is freed; in this case it can no more be used again.
> > > >
> > > > the attached patch fixes this, by making the uaudio_close() release
> > > > the as_info structures in use.
> > > >
> > > > to test the patch, open and immediately close the uaudio device,
> > > > for instance:
> > > >
> > > > cat /dev/null >/dev/audio
> > > >
> > > > this will allocate the play.rate=8000 setting, but will not free it.
> > > > Then try to set the play.rate parameter to its current value:
> > > >
> > > > audioctl play.rate=8000
> > > >
> > > > without the patch, audioctl(1) will fail because the parameter is
> > > > still tagged as busy.
> > >
> > > would not calling uaudio_chan_close() be the right thing then?
> > >
> >
> > uaudio_chan_close() should be called in order to close a chan opened
> > with uaudio_chan_open(). Chans are opened when input or output is
> > triggered.
> >
> > However, parameters set (struct as_info) may be tagged as busy
> > without any channel being opened (so no chan to close). For instance,
> > this may happen if the device is opened and then closed (no read() or
> > write() is called).
>
> uaudio_chan_close() deals olrite w/ deallocating all the resources
> if needed as well as resetting the busy flag.
> it is more consistant than just resetting the flag w/o checking
> that all the resources are not allocated as well.
> so how about this diff?
>

it works. I didn't notice anything "strange".  
uaudio_chan_close() calls usbd_set_interface(0) and this seems ok.

(tested with a m-audio mobilepre on i386)

--
Alexandre