Messages in this thread Patch in this message | | | Date | Sat, 10 Feb 2001 22:54:28 -0500 | From | Jeff Garzik <> | Subject | Re: [PATCH] More network pci_enable cleanups. |
| |
davej@suse.de wrote: > diff -urN --exclude-from=/home/davej/.exclude linux/drivers/net/defxx.c linux-dj/drivers/net/defxx.c > --- linux/drivers/net/defxx.c Sat Feb 10 02:49:43 2001 > +++ linux-dj/drivers/net/defxx.c Sat Feb 10 03:05:03 2001 > @@ -414,6 +415,7 @@ > struct net_device *dev; > DFX_board_t *bp; /* board pointer */ > static int version_disp; > + int err; > > if (!version_disp) /* display version info if adapter is found */ > { > @@ -429,8 +431,8 @@ > > /* Enable PCI device. */ > if (pdev != NULL) { > - if (pci_enable_device (pdev)) > - goto err_out; > + err = pci_enable_device (pdev); > + if (err) return err; > ioaddr = pci_resource_start (pdev, 1); > }
1) Bug: Introduced memory leak by replacing "goto err_out" with "return err"
2) Sole usage of 'err' -- it should be scoped inside the pdev!=NULL check.
> diff -urN --exclude-from=/home/davej/.exclude linux/drivers/net/dgrs.c linux-dj/drivers/net/dgrs.c > --- linux/drivers/net/dgrs.c Sat Feb 10 02:49:43 2001 > +++ linux-dj/drivers/net/dgrs.c Sat Feb 10 03:05:09 2001 > @@ -1352,7 +1355,7 @@ > > static int __init dgrs_scan(void) > { > - int cards_found = 0; > + int cards_found; > uint io; > uint mem; > uint irq;
Rejected. Introduces bug. That zero is required!
> diff -urN --exclude-from=/home/davej/.exclude linux/drivers/net/eepro.c linux-dj/drivers/net/eepro.c > --- linux/drivers/net/eepro.c Sat Feb 10 02:49:43 2001 > +++ linux-dj/drivers/net/eepro.c Sat Feb 10 03:05:05 2001 > @@ -1098,7 +1098,7 @@ > printk (KERN_ERR "%s: transmit timed out, %s?\n", dev->name, > "network cable problem"); > /* This is not a duplicate. One message for the console, > - one for the the log file */ > + one for the log file */ > printk (KERN_DEBUG "%s: transmit timed out, %s?\n", dev->name, > "network cable problem"); > eepro_complete_selreset(ioaddr);
rejected -- already in AC patches (and merged into my tree as well)
> diff -urN --exclude-from=/home/davej/.exclude linux/drivers/net/eepro100.c linux-dj/drivers/net/eepro100.c > --- linux/drivers/net/eepro100.c Sat Feb 10 02:49:43 2001 > +++ linux-dj/drivers/net/eepro100.c Sat Feb 10 03:05:06 2001 > @@ -550,10 +552,10 @@ > { > unsigned long ioaddr; > int irq, i; > - int acpi_idle_state = 0, pm; > - static int cards_found /* = 0 */; > + int acpi_idle_state, pm; > + static int cards_found; > + static int did_version; /* Already printed version info. */ > > - static int did_version /* = 0 */; /* Already printed version info. */ > if (speedo_debug > 0 && did_version++ == 0) > printk(version); >
Rejected. The maintainer clearly wanted the "/* = 0 */" present, yet you remove them. I'm suspicious of the acpi_idle_state initialization removal as well, but can't check it quickly at present (acpi_idle_state is gone in my tree)
> diff -urN --exclude-from=/home/davej/.exclude linux/drivers/net/ncr885e.c linux-dj/drivers/net/ncr885e.c > --- linux/drivers/net/ncr885e.c Sat Feb 10 02:49:44 2001 > +++ linux-dj/drivers/net/ncr885e.c Sat Feb 10 03:05:05 2001 > @@ -1163,7 +1163,7 @@ > } > > /* Since the NCR 53C885 is a multi-function chip, I'm not worrying about > - * trying to get the the device(s) in slot order. For our (Synergy's) > + * trying to get the device(s) in slot order. For our (Synergy's) > * purpose, there's just a single 53C885 on the board and we don't > * worry about the rest. > */
rejected -- already in AC and my own patches
All the rest were applied.
Jeff
-- Jeff Garzik | "You see, in this world there's two kinds of Building 1024 | people, my friend: Those with loaded guns MandrakeSoft | and those who dig. You dig." --Blondie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
| |