* Re: [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 [not found] <E1D8x4I-0002D2-PI@sc8-pr-cvs1.sourceforge.net> @ 2005-03-09 9:05 ` Takashi Iwai 2005-03-09 9:20 ` Clemens Ladisch 2005-03-09 20:22 ` Lee Revell 0 siblings, 2 replies; 5+ messages in thread From: Takashi Iwai @ 2005-03-09 9:05 UTC (permalink / raw) To: Clemens Ladisch; +Cc: alsa-devel At Wed, 09 Mar 2005 01:01:26 -0800, Clemens Ladisch wrote: > > Update of /cvsroot/alsa/alsa-kernel/core/oss > In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8486 > > Modified Files: > mixer_oss.c > Log Message: > Summary: fix forgotten release of semaphore in error path > > In snd_mixer_oss_get_volume1_vol and snd_mixer_oss_put_volume1_vol, > card->controls_rwsem wouldn't be released if the boolean type > check fails. IMO, it'd be better to use a plain if() rather than this macro. It lowers readability. Takashi > > Index: mixer_oss.c > =================================================================== > RCS file: /cvsroot/alsa/alsa-kernel/core/oss/mixer_oss.c,v > retrieving revision 1.38 > retrieving revision 1.39 > diff -u -r1.38 -r1.39 > --- mixer_oss.c 20 Jan 2005 17:14:44 -0000 1.38 > +++ mixer_oss.c 9 Mar 2005 09:01:23 -0000 1.39 > @@ -522,7 +522,7 @@ > goto __unalloc; > snd_runtime_check(!kctl->info(kctl, uinfo), goto __unalloc); > snd_runtime_check(!kctl->get(kctl, uctl), goto __unalloc); > - snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, return); > + snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, goto __unalloc); > *left = snd_mixer_oss_conv1(uctl->value.integer.value[0], uinfo->value.integer.min, uinfo->value.integer.max, &pslot->volume[0]); > if (uinfo->count > 1) > *right = snd_mixer_oss_conv1(uctl->value.integer.value[1], uinfo->value.integer.min, uinfo->value.integer.max, &pslot->volume[1]); > @@ -616,7 +616,7 @@ > if (uinfo == NULL || uctl == NULL) > goto __unalloc; > snd_runtime_check(!kctl->info(kctl, uinfo), goto __unalloc); > - snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, return); > + snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, goto __unalloc); > uctl->value.integer.value[0] = snd_mixer_oss_conv2(left, uinfo->value.integer.min, uinfo->value.integer.max); > if (uinfo->count > 1) > uctl->value.integer.value[1] = snd_mixer_oss_conv2(right, uinfo->value.integer.min, uinfo->value.integer.max); > > > > ------------------------------------------------------- > SF email is sponsored by - The IT Product Guide > Read honest & candid reviews on hundreds of IT Products from real users. > Discover which products truly live up to the hype. Start reading now. > http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click > _______________________________________________ > Alsa-cvslog mailing list > Alsa-cvslog@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/alsa-cvslog > ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 2005-03-09 9:05 ` [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 Takashi Iwai @ 2005-03-09 9:20 ` Clemens Ladisch 2005-03-09 9:36 ` Takashi Iwai 2005-03-09 20:22 ` Lee Revell 1 sibling, 1 reply; 5+ messages in thread From: Clemens Ladisch @ 2005-03-09 9:20 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel Takashi Iwai wrote: > Clemens Ladisch wrote: > > Summary: fix forgotten release of semaphore in error path > > > > In snd_mixer_oss_get_volume1_vol and snd_mixer_oss_put_volume1_vol, > > card->controls_rwsem wouldn't be released if the boolean type > > check fails. > > ... > > - snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, return); > > + snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, goto __unalloc); > > IMO, it'd be better to use a plain if() rather than this macro. > It lowers readability. Yes, these lines are too long. I made the minimal change to make it correct, just to be sure. Feel free to change it any way you want. :-) BTW: are there any plans to enforce the 80 columns limit of Linux source code? ;-) Regards, Clemens ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 2005-03-09 9:20 ` Clemens Ladisch @ 2005-03-09 9:36 ` Takashi Iwai 0 siblings, 0 replies; 5+ messages in thread From: Takashi Iwai @ 2005-03-09 9:36 UTC (permalink / raw) To: Clemens Ladisch; +Cc: alsa-devel At Wed, 9 Mar 2005 10:20:58 +0100 (MET), Clemens Ladisch wrote: > > Takashi Iwai wrote: > > Clemens Ladisch wrote: > > > Summary: fix forgotten release of semaphore in error path > > > > > > In snd_mixer_oss_get_volume1_vol and snd_mixer_oss_put_volume1_vol, > > > card->controls_rwsem wouldn't be released if the boolean type > > > check fails. > > > ... > > > - snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, return); > > > + snd_runtime_check(uinfo->type != SNDRV_CTL_ELEM_TYPE_BOOLEAN || uinfo->value.integer.min != 0 || uinfo->value.integer.max != 1, goto __unalloc); > > > > IMO, it'd be better to use a plain if() rather than this macro. > > It lowers readability. > > Yes, these lines are too long. > > I made the minimal change to make it correct, just to be sure. Feel > free to change it any way you want. :-) Hehe, I'm a broad-minded man, glad to give you a chance, too ;) > BTW: are there any plans to enforce the 80 columns limit of Linux > source code? ;-) Let's apply "indent -kr -i8 -l80" for all files :) Well, the problem of snd_runtime_check() is not its length but that it prevents to understand the code flow, just as you found in thie case. Takashi ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 2005-03-09 9:05 ` [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 Takashi Iwai 2005-03-09 9:20 ` Clemens Ladisch @ 2005-03-09 20:22 ` Lee Revell 2005-03-10 9:42 ` Clemens Ladisch 1 sibling, 1 reply; 5+ messages in thread From: Lee Revell @ 2005-03-09 20:22 UTC (permalink / raw) To: Takashi Iwai; +Cc: Clemens Ladisch, alsa-devel On Wed, 2005-03-09 at 10:05 +0100, Takashi Iwai wrote: > At Wed, 09 Mar 2005 01:01:26 -0800, > Clemens Ladisch wrote: > > > > Update of /cvsroot/alsa/alsa-kernel/core/oss > > In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv8486 > > > > Modified Files: > > mixer_oss.c > > Log Message: > > Summary: fix forgotten release of semaphore in error path > > > > In snd_mixer_oss_get_volume1_vol and snd_mixer_oss_put_volume1_vol, > > card->controls_rwsem wouldn't be released if the boolean type > > check fails. Clemens, should this patch fix OSDL bug 4282? You mentioned that was related to a forgotten semaphore release in the same code. Lee ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 2005-03-09 20:22 ` Lee Revell @ 2005-03-10 9:42 ` Clemens Ladisch 0 siblings, 0 replies; 5+ messages in thread From: Clemens Ladisch @ 2005-03-10 9:42 UTC (permalink / raw) To: Lee Revell; +Cc: alsa-devel Lee Revell wrote: > > Clemens Ladisch wrote: > > > Summary: fix forgotten release of semaphore in error path > > > > > > In snd_mixer_oss_get_volume1_vol and snd_mixer_oss_put_volume1_vol, > > > card->controls_rwsem wouldn't be released if the boolean type > > > check fails. > > Clemens, should this patch fix OSDL bug 4282? You mentioned that was > related to a forgotten semaphore release in the same code. Neither this fix nor my earlier one in snd_mixer_oss_build_input looks like it could be related to that bug. A semaphore that wasn't released would result in a deadlock, not in a crash. I would guess that the bug is caused by using something that hasn't been initialized yet. Regards, Clemens ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2005-03-10 9:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <E1D8x4I-0002D2-PI@sc8-pr-cvs1.sourceforge.net>
2005-03-09 9:05 ` [alsa-cvslog] CVS: alsa-kernel/core/oss mixer_oss.c,1.38,1.39 Takashi Iwai
2005-03-09 9:20 ` Clemens Ladisch
2005-03-09 9:36 ` Takashi Iwai
2005-03-09 20:22 ` Lee Revell
2005-03-10 9:42 ` Clemens Ladisch
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.