[Kernel-janitors] [PATHC] fix ide.c warning

[Kernel-janitors] [PATHC] fix ide.c warning

[Don Koch]

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

This fixes the "implicit declaration of function `pnpide_init'" warning
in ide.c.  Based on 2.6.5-mm5.

===== drivers/ide/ide.c 1.132 vs edited =====
--- 1.132/drivers/ide/ide.c	Sat Apr 10 18:22:11 2004
+++ edited/drivers/ide/ide.c	Tue Apr 13 20:59:52 2004
@@ -2245,7 +2245,10 @@
 		return 1;
 	}
 #if defined(CONFIG_BLK_DEV_IDEPNP) && defined(CONFIG_PNP) && defined(MODULE)
-	pnpide_init(0);
+	{
+		extern void pnpide_init(int enable);
+		pnpide_init(0);
+	}
 #endif /* CONFIG_BLK_DEV_IDEPNP */
 #ifdef CONFIG_PROC_FS
 	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);



[-- 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

Re: [Kernel-janitors] [PATHC] fix ide.c warning

[Luiz Fernando N. Capitulino]

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


Hi Don,

Em Tue, 13 Apr 2004 22:17:53 -0400
Don Koch <aardvark@krl.com> escreveu:

| This fixes the "implicit declaration of function `pnpide_init'" warning
| in ide.c.  Based on 2.6.5-mm5.
| 
| ===== drivers/ide/ide.c 1.132 vs edited =====
| --- 1.132/drivers/ide/ide.c	Sat Apr 10 18:22:11 2004
| +++ edited/drivers/ide/ide.c	Tue Apr 13 20:59:52 2004
| @@ -2245,7 +2245,10 @@
|  		return 1;
|  	}
|  #if defined(CONFIG_BLK_DEV_IDEPNP) && defined(CONFIG_PNP) && defined(MODULE)
| -	pnpide_init(0);
| +	{
| +		extern void pnpide_init(int enable);
| +		pnpide_init(0);
| +	}
|  #endif /* CONFIG_BLK_DEV_IDEPNP */
|  #ifdef CONFIG_PROC_FS
|  	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);

 what about to declare pnpide_init() in include/linux/ide.h (like ide_end_request),
and just call it ? It is a bad idea ?

-- 
Luiz Fernando N. Capitulino
<lcapitulino@prefeitura.sp.gov.br>
<http://www.telecentros.sp.gov.br>

[-- 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

Re: [Kernel-janitors] [PATHC] fix ide.c warning

[Michael Still]

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

Luiz Fernando N. Capitulino wrote:
> Hi Don,
> 
> Em Tue, 13 Apr 2004 22:17:53 -0400
> Don Koch <aardvark@krl.com> escreveu:
> 
> | This fixes the "implicit declaration of function `pnpide_init'" warning
> | in ide.c.  Based on 2.6.5-mm5.
> | 
> | ===== drivers/ide/ide.c 1.132 vs edited =====
> | --- 1.132/drivers/ide/ide.c	Sat Apr 10 18:22:11 2004
> | +++ edited/drivers/ide/ide.c	Tue Apr 13 20:59:52 2004
> | @@ -2245,7 +2245,10 @@
> |  		return 1;
> |  	}
> |  #if defined(CONFIG_BLK_DEV_IDEPNP) && defined(CONFIG_PNP) && defined(MODULE)
> | -	pnpide_init(0);
> | +	{
> | +		extern void pnpide_init(int enable);
> | +		pnpide_init(0);
> | +	}
> |  #endif /* CONFIG_BLK_DEV_IDEPNP */
> |  #ifdef CONFIG_PROC_FS
> |  	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);
> 
>  what about to declare pnpide_init() in include/linux/ide.h (like ide_end_request),
> and just call it ? It is a bad idea ?

I would think it's much better form to either:

  - put the prototype in a header
  - repeat the prototype at the top of this file (there are risks here)

With the first one being much prefered in my book. It's just not 
idiomatic in C to do it this way. You also don't need the extern:

      void pnpide_init(int enable);

should be sufficient.

Additionally, putting that block of code in braces is evil.

Cheers,
Mikal

PS: I didn't look at the definition of pnpide_init, and assume it's not 
static or something.

[-- 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

Re: [Kernel-janitors] [PATHC] fix ide.c warning

[Don Koch]

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

mikal@stillhq.com said:
> Luiz Fernando N. Capitulino wrote:
> > Hi Don,
> > 
> > Em Tue, 13 Apr 2004 22:17:53 -0400
> > Don Koch <aardvark@krl.com> escreveu:
> > 
> > | This fixes the "implicit declaration of function `pnpide_init'" warning
> > | in ide.c.  Based on 2.6.5-mm5.
> > | 
> > | ===== drivers/ide/ide.c 1.132 vs edited =====
> > | --- 1.132/drivers/ide/ide.c	Sat Apr 10 18:22:11 2004
> > | +++ edited/drivers/ide/ide.c	Tue Apr 13 20:59:52 2004
> > | @@ -2245,7 +2245,10 @@
> > |  		return 1;
> > |  	}
> > |  #if defined(CONFIG_BLK_DEV_IDEPNP) && defined(CONFIG_PNP) && defined(MODULE)
> > | -	pnpide_init(0);
> > | +	{
> > | +		extern void pnpide_init(int enable);
> > | +		pnpide_init(0);
> > | +	}
> > |  #endif /* CONFIG_BLK_DEV_IDEPNP */
> > |  #ifdef CONFIG_PROC_FS
> > |  	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);
> > 
> >  what about to declare pnpide_init() in include/linux/ide.h (like ide_end_request),
> > and just call it ? It is a bad idea ?
> 
> I would think it's much better form to either:
> 
>   - put the prototype in a header
>   - repeat the prototype at the top of this file (there are risks here)
> 
> With the first one being much prefered in my book. It's just not 
> idiomatic in C to do it this way. You also don't need the extern:
> 
>       void pnpide_init(int enable);
> 
> should be sufficient.
> 
> Additionally, putting that block of code in braces is evil.
> 
> Cheers,
> Mikal
> 
> PS: I didn't look at the definition of pnpide_init, and assume it's not 
> static or something.
> 

The patch echos other calls to pnpide_init() in the same file.
The evil was done before I got there.  (I agree, it's ugly.)
I'll poke around and see if I can find a more appropriate way
to handle all of the declarations and submit a follow-up patch.

-- 
Don Koch
http://www.krl.com/



[-- 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

Re: [Kernel-janitors] [PATHC] fix ide.c warning

[Luiz Fernando N. Capitulino]

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


 Hi Michael,

Em Thu, 15 Apr 2004 07:10:02 +1000
Michael Still <mikal@stillhq.com> escreveu:

| > | This fixes the "implicit declaration of function `pnpide_init'" warning
| > | in ide.c.  Based on 2.6.5-mm5.
| > | 
| > | ===== drivers/ide/ide.c 1.132 vs edited =====
| > | --- 1.132/drivers/ide/ide.c	Sat Apr 10 18:22:11 2004
| > | +++ edited/drivers/ide/ide.c	Tue Apr 13 20:59:52 2004
| > | @@ -2245,7 +2245,10 @@
| > |  		return 1;
| > |  	}
| > |  #if defined(CONFIG_BLK_DEV_IDEPNP) && defined(CONFIG_PNP) && defined(MODULE)
| > | -	pnpide_init(0);
| > | +	{
| > | +		extern void pnpide_init(int enable);
| > | +		pnpide_init(0);
| > | +	}
| > |  #endif /* CONFIG_BLK_DEV_IDEPNP */
| > |  #ifdef CONFIG_PROC_FS
| > |  	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);
| > 
| >  what about to declare pnpide_init() in include/linux/ide.h (like ide_end_request),
| > and just call it ? It is a bad idea ?
| 
| I would think it's much better form to either:
| 
|   - put the prototype in a header
|   - repeat the prototype at the top of this file (there are risks here)

 I don't think is a good way to create a new .h file only to put
one prototype there. include/linux/ide.h already does that for another ide
functions, so it can be used (well, was what a quick look show me).

| With the first one being much prefered in my book. It's just not 
| idiomatic in C to do it this way. You also don't need the extern:
| 
|       void pnpide_init(int enable);
| 
| should be sufficient.

 IMHO, the extern declaration makes it easy to understand.

| Additionally, putting that block of code in braces is evil.

 Yes, is what I'm trying to avoid (but I already added evil code
in the kernel :-)).


-- 
Luiz Fernando N. Capitulino
<lcapitulino@prefeitura.sp.gov.br>
<http://www.telecentros.sp.gov.br>

[-- 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

Re: [Kernel-janitors] [PATHC] fix ide.c warning

[Luiz Fernando N. Capitulino]

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

Em Wed, 14 Apr 2004 17:17:45 -0400
Don Koch <aardvark@krl.com> escreveu:

| > > Em Tue, 13 Apr 2004 22:17:53 -0400
| > > Don Koch <aardvark@krl.com> escreveu:
| > > 
| > > | This fixes the "implicit declaration of function `pnpide_init'" warning
| > > | in ide.c.  Based on 2.6.5-mm5.
| > > | 
| > > | ===== drivers/ide/ide.c 1.132 vs edited =====
| > > | --- 1.132/drivers/ide/ide.c	Sat Apr 10 18:22:11 2004
| > > | +++ edited/drivers/ide/ide.c	Tue Apr 13 20:59:52 2004
| > > | @@ -2245,7 +2245,10 @@
| > > |  		return 1;
| > > |  	}
| > > |  #if defined(CONFIG_BLK_DEV_IDEPNP) && defined(CONFIG_PNP) && defined(MODULE)
| > > | -	pnpide_init(0);
| > > | +	{
| > > | +		extern void pnpide_init(int enable);
| > > | +		pnpide_init(0);
| > > | +	}
| > > |  #endif /* CONFIG_BLK_DEV_IDEPNP */
| > > |  #ifdef CONFIG_PROC_FS
| > > |  	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);
| > > 
| > >  what about to declare pnpide_init() in include/linux/ide.h (like ide_end_request),
| > > and just call it ? It is a bad idea ?
| > 
| > I would think it's much better form to either:
| > 
| >   - put the prototype in a header
| >   - repeat the prototype at the top of this file (there are risks here)
| > 
| > With the first one being much prefered in my book. It's just not 
| > idiomatic in C to do it this way. You also don't need the extern:
| > 
| >       void pnpide_init(int enable);
| > 
| > should be sufficient.
| > 
| > Additionally, putting that block of code in braces is evil.
| > 
| > Cheers,
| > Mikal
| > 
| > PS: I didn't look at the definition of pnpide_init, and assume it's not 
| > static or something.
| > 
| 
| The patch echos other calls to pnpide_init() in the same file.
| The evil was done before I got there.  (I agree, it's ugly.)
| I'll poke around and see if I can find a more appropriate way
| to handle all of the declarations and submit a follow-up patch.

 Will be very good if we remove the warning with a good way.


-- 
Luiz Fernando N. Capitulino
<lcapitulino@prefeitura.sp.gov.br>
<http://www.telecentros.sp.gov.br>

[-- 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

Re: [Kernel-janitors] [PATHC] fix ide.c warning

[Michael Still]

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

Luiz Fernando N. Capitulino wrote:

> | I would think it's much better form to either:
> | 
> |   - put the prototype in a header
> |   - repeat the prototype at the top of this file (there are risks here)
> 
>  I don't think is a good way to create a new .h file only to put
> one prototype there. include/linux/ide.h already does that for another ide
> functions, so it can be used (well, was what a quick look show me).

Hey, I don't really mind which header it goes in. If there is an 
existing suitable header, then that sounds like a good path.

Cheers,
Mikal

-- 

Michael Still (mikal@stillhq.com) | "All my life I've had one dream,
http://www.stillhq.com            |  to achieve my many goals"
UTC + 11                          |    -- Homer Simpson

[-- 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