[2.6 patch] fix a drivers/char/isicom.c compile warning

[2.6 patch] fix a drivers/char/isicom.c compile warning

[Adrian Bunk]

I got the following compile warning in 2.6.1-mm2 (but it doesn't seem to 
be specific to -mm):


<--  snip  -->

...
  CC [M]  drivers/char/isicom.o
...
drivers/char/isicom.c: In function `unregister_drivers':
drivers/char/isicom.c:1677: warning: `error' might be used uninitialized in this function
...

<--  snip  -->


The following patch fixes this issue:


--- linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c.old	2004-01-13 00:40:02.000000000 +0100
+++ linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c	2004-01-13 00:49:00.000000000 +0100
@@ -1675,7 +1675,7 @@
 static void unregister_drivers(void)
 {
 	int error;
-	if (tty_unregister_driver(isicom_normal))
+	if ((error=tty_unregister_driver(isicom_normal)))
 		printk(KERN_DEBUG "ISICOM: couldn't unregister normal driver error=%d.\n",error);
 	put_tty_driver(isicom_normal);
 }



cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] fix a drivers/char/isicom.c compile warning

[Arnaldo Carvalho de Melo]

Em Tue, Jan 13, 2004 at 01:00:56AM +0100, Adrian Bunk escreveu:
> I got the following compile warning in 2.6.1-mm2 (but it doesn't seem to 
> be specific to -mm):
> 
> 
> <--  snip  -->
> 
> ...
>   CC [M]  drivers/char/isicom.o
> ...
> drivers/char/isicom.c: In function `unregister_drivers':
> drivers/char/isicom.c:1677: warning: `error' might be used uninitialized in this function
> ...
> 
> <--  snip  -->
> 
> 
> The following patch fixes this issue:
> 
> 
> --- linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c.old	2004-01-13 00:40:02.000000000 +0100
> +++ linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c	2004-01-13 00:49:00.000000000 +0100
> @@ -1675,7 +1675,7 @@
>  static void unregister_drivers(void)
>  {
>  	int error;
> -	if (tty_unregister_driver(isicom_normal))
> +	if ((error=tty_unregister_driver(isicom_normal)))
>  		printk(KERN_DEBUG "ISICOM: couldn't unregister normal driver error=%d.\n",error);

OK, the patch is right, but couldn't we take the opportunity to make this
more readable while at it? Ssomething like:

static void unregister_drivers(void)
{
        int error = tty_unregister_driver(isicom_normal);

        if (error)
		printk(KERN_DEBUG "ISICOM: couldn't unregister normal "
				  "driver error=%d.\n", error);

? :-)

- Arnaldo

Re: [2.6 patch] fix a drivers/char/isicom.c compile warning

[Adrian Bunk]

On Mon, Jan 12, 2004 at 10:17:46PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 13, 2004 at 01:00:56AM +0100, Adrian Bunk escreveu:
>...
> > The following patch fixes this issue:
> > 
> > 
> > --- linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c.old	2004-01-13 00:40:02.000000000 +0100
> > +++ linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c	2004-01-13 00:49:00.000000000 +0100
> > @@ -1675,7 +1675,7 @@
> >  static void unregister_drivers(void)
> >  {
> >  	int error;
> > -	if (tty_unregister_driver(isicom_normal))
> > +	if ((error=tty_unregister_driver(isicom_normal)))
> >  		printk(KERN_DEBUG "ISICOM: couldn't unregister normal driver error=%d.\n",error);
> 
> OK, the patch is right, but couldn't we take the opportunity to make this
> more readable while at it? Ssomething like:
> 
> static void unregister_drivers(void)
> {
>         int error = tty_unregister_driver(isicom_normal);
> 
>         if (error)
> 		printk(KERN_DEBUG "ISICOM: couldn't unregister normal "
> 				  "driver error=%d.\n", error);
> 
> ? :-)

I thought about it, but I wasn't sure whether changing a driver to be 
more readable in _one_ place and making the code there different from 
the coding style used in the rest of the driver is really an 
improvement (and no, I don't want to clean the whole driver...)?

> - Arnaldo

cu
Adrian

--- linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c.old	2004-01-13 00:40:02.000000000 +0100
+++ linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c	2004-01-13 01:21:15.000000000 +0100
@@ -1674,9 +1674,11 @@
 
 static void unregister_drivers(void)
 {
-	int error;
-	if (tty_unregister_driver(isicom_normal))
+	int error = tty_unregister_driver(isicom_normal);
+
+	if (error)
 		printk(KERN_DEBUG "ISICOM: couldn't unregister normal driver error=%d.\n",error);
+
 	put_tty_driver(isicom_normal);
 }
 

Re: [2.6 patch] fix a drivers/char/isicom.c compile warning

[Arnaldo Carvalho de Melo]

Em Tue, Jan 13, 2004 at 01:23:18AM +0100, Adrian Bunk escreveu:
> On Mon, Jan 12, 2004 at 10:17:46PM -0200, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jan 13, 2004 at 01:00:56AM +0100, Adrian Bunk escreveu:
> >...
> > > The following patch fixes this issue:
> > > 
> > > 
> > > --- linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c.old	2004-01-13 00:40:02.000000000 +0100
> > > +++ linux-2.6.1-mm2-modular-no-smp/drivers/char/isicom.c	2004-01-13 00:49:00.000000000 +0100
> > > @@ -1675,7 +1675,7 @@
> > >  static void unregister_drivers(void)
> > >  {
> > >  	int error;
> > > -	if (tty_unregister_driver(isicom_normal))
> > > +	if ((error=tty_unregister_driver(isicom_normal)))
> > >  		printk(KERN_DEBUG "ISICOM: couldn't unregister normal driver error=%d.\n",error);
> > 
> > OK, the patch is right, but couldn't we take the opportunity to make this
> > more readable while at it? Ssomething like:
> > 
> > static void unregister_drivers(void)
> > {
> >         int error = tty_unregister_driver(isicom_normal);
> > 
> >         if (error)
> > 		printk(KERN_DEBUG "ISICOM: couldn't unregister normal "
> > 				  "driver error=%d.\n", error);
> > 
> > ? :-)
> 
> I thought about it, but I wasn't sure whether changing a driver to be 
> more readable in _one_ place and making the code there different from 
> the coding style used in the rest of the driver is really an 
> improvement (and no, I don't want to clean the whole driver...)?

Humm, IMHO it is better to get it with mixed style as long as the changes that
"break" the style are for the correct style, but this, of course, for purely
unmaintained stuff, like... isicom.c :-) For maintained stuff it becomes a
pain in the ass for the maintainer, that should try to follow CodingStyle, but
as we know, not always do.
 

Re: [2.6 patch] fix a drivers/char/isicom.c compile warning

[Angelo Dell'Aera]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 12 Jan 2004 22:38:28 -0200
Arnaldo Carvalho de Melo <acme@conectiva.com.br> wrote:

>> I thought about it, but I wasn't sure whether changing a driver to be 
>> more readable in _one_ place and making the code there different from 
>> the coding style used in the rest of the driver is really an 
>> improvement (and no, I don't want to clean the whole driver...)?
>
>Humm, IMHO it is better to get it with mixed style as long as the changes that
>"break" the style are for the correct style, but this, of course, for purely
>unmaintained stuff, like... isicom.c :-) For maintained stuff it becomes a
>pain in the ass for the maintainer, that should try to follow CodingStyle, but
>as we know, not always do.

I started cleaning isicom.c in such a way as to be more "CodingStyle 
compliant" but it's a hard pain... the fix from Adrian is still in.
Hope to release it in a reasonable time.

Regards. 

- --

Angelo Dell'Aera 'buffer' 
Antifork Research, Inc.	  	http://buffer.antifork.org

PGP information in e-mail header


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFAA9ptpONIzxnBXKIRAhlqAJ9jefAfKxXeLJYb0SCpqBp8EGfN0QCfdvuo
ToB15Ba8cV73AMOpbcjAe4I=
=fXgy
-----END PGP SIGNATURE-----