[Kernel-janitors] Re: request_region() on sound/oss/wavfront.c.

[Kernel-janitors] Re: request_region() on sound/oss/wavfront.c.

[Domen Puncer]

[-- Attachment #1: Type: text/plain, Size: 835 bytes --]

On 22/05/04 18:54 -0300, Gustavo Franco wrote:
> Hi list,
> 
> [Patch aplied against 2.6.6-bk9 - compiles cleanly. ]
> 
> I've added two error checks for request_region() calls on the source
> and replaced a check_region() with request_region().The release_region()
> calls seems to be ok, but let me known if i'm missing something.
> 
> Comments?
>

You request same region twice...

> +++ sound/oss/wavfront.c  2004-05-22 18:28:38.000000000 -0300
> +       if (!request_region (io_base, 16, "wavefront")) {
Few lines later there is a:
	dev.base = io_base;
So this will print ugly error messages.

> +       if(!request_region (dev.base+2, 6, "wavefront synth")) {
> +               if(!request_region (dev.base+8, 8, "wavefront fx")) {

IMHO, you can just get rid of latter 2 request_region's.
And don't forget about release_region.

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[Kernel-janitors] Re: request_region() on sound/oss/wavfront.c.

[Domen Puncer]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]

On 23/05/04 00:43 +0200, Domen Puncer wrote:
> On 22/05/04 18:54 -0300, Gustavo Franco wrote:
> 
> > +++ sound/oss/wavfront.c  2004-05-22 18:28:38.000000000 -0300
> > +       if (!request_region (io_base, 16, "wavefront")) {
> Few lines later there is a:
> 	dev.base = io_base;
> So this will print ugly error messages.
> 
> > +       if(!request_region (dev.base+2, 6, "wavefront synth")) {
> > +               if(!request_region (dev.base+8, 8, "wavefront fx")) {
> 
> IMHO, you can just get rid of latter 2 request_region's.

Ehm, no, damn. Comment above check_region states that device can take
up 8 ports.

So if there is another device on base+8 this wont work correctly (if that
device registred it's region before, even the old code wont work ok).


> And don't forget about release_region.


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[Kernel-janitors] Re: request_region() on sound/oss/wavfront.c.

[Gustavo Franco]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

Domen Puncer wrote:

>On 23/05/04 00:43 +0200, Domen Puncer wrote:
>  
>
>>On 22/05/04 18:54 -0300, Gustavo Franco wrote:
>>
>>    
>>
>>>+++ sound/oss/wavfront.c  2004-05-22 18:28:38.000000000 -0300
>>>+       if (!request_region (io_base, 16, "wavefront")) {
>>>      
>>>
>>Few lines later there is a:
>>	dev.base = io_base;
>>So this will print ugly error messages.
>>    
>>
Sorry, but do you mean KERN_ERR ones?

>>    
>>
>>>+       if(!request_region (dev.base+2, 6, "wavefront synth")) {
>>>+               if(!request_region (dev.base+8, 8, "wavefront fx")) {
>>>      
>>>
>>IMHO, you can just get rid of latter 2 request_region's.
>>    
>>
>
>Ehm, no, damn. Comment above check_region states that device can take
>up 8 ports.
>
>So if there is another device on base+8 this wont work correctly (if that
>device registred it's region before, even the old code wont work ok).
>
>  
>
What? Can you summarize your two messages? Check that both 
request_region() cited
by you above were not added by me, i've added a return check only.


Thanks,
--
Gustavo Franco

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[Kernel-janitors] Re: request_region() on sound/oss/wavfront.c.

[Domen Puncer]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On 22/05/04 22:12 -0300, Gustavo Franco wrote:
> Domen Puncer wrote:
> 
> >On 23/05/04 00:43 +0200, Domen Puncer wrote:
> > 
> >
> >>On 22/05/04 18:54 -0300, Gustavo Franco wrote:
> >>
> >>   
> >>
> >>>+++ sound/oss/wavfront.c  2004-05-22 18:28:38.000000000 -0300
> >>>+       if (!request_region (io_base, 16, "wavefront")) {
> >>>     
> >>>
> >>Few lines later there is a:
> >>	dev.base = io_base;
> >>So this will print ugly error messages.
> >>   
> >>
> Sorry, but do you mean KERN_ERR ones?

I mean error messeges below request_region's that will fail.

 
> >>   
> >>
> >>>+       if(!request_region (dev.base+2, 6, "wavefront synth")) {
> >>>+               if(!request_region (dev.base+8, 8, "wavefront fx")) {
> >>>     
> >>>
> >>IMHO, you can just get rid of latter 2 request_region's.
> >>   
> >>
> >
> >Ehm, no, damn. Comment above check_region states that device can take
> >up 8 ports.
> >
> >So if there is another device on base+8 this wont work correctly (if that
> >device registred it's region before, even the old code wont work ok).
> >
> > 
> >
> What? Can you summarize your two messages? Check that both 
> request_region() cited
> by you above were not added by me, i've added a return check only.

Yes, i know that, but the request_region you added causes the other two
to fail. (You can't request same region twice)
It seems right to me to request_region(base, 8) where check_region is
now, and another (base+8, 8) before fx detection.

> 
> 
> Thanks,
> --
> Gustavo Franco

> _______________________________________________
> Kernel-janitors mailing list
> Kernel-janitors@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/kernel-janitors


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors