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
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
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.
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);
}
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Mon, 12 Jan 2004 22:38:28 -0200
Arnaldo Carvalho de Melo <[email protected]> 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-----