lkml.org 
[lkml]   [2004]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] ibmasm: add missing pci_enable_device()
Bjorn Helgaas <bjorn.helgaas@hp.com> :
[...]
> > > ===== drivers/misc/ibmasm/module.c 1.2 vs edited =====
> > > --- 1.2/drivers/misc/ibmasm/module.c 2004-05-14 06:00:50 -06:00
> > > +++ edited/drivers/misc/ibmasm/module.c 2004-08-04 13:15:46 -06:00
> > > @@ -62,10 +62,17 @@
> > > int result = -ENOMEM;
> > > struct service_processor *sp;
> > >
> > > + if (pci_enable_device(pdev)) {
> > > + printk(KERN_ERR "%s: can't enable PCI device at %s\n",
> > > + DRIVER_NAME, pci_name(pdev));
> > > + return -ENODEV;

Btw, pci_enable_device returns a perfectly fine status code. There is no
reason to override it ('result = pci_enable_device(...' and you can even
remove the previous ENOMEM initialization).

[...]
> > > sp = kmalloc(sizeof(struct service_processor), GFP_KERNEL);
> > > if (sp == NULL) {
> > > dev_err(&pdev->dev, "Failed to allocate memory\n");
> > > - return result;
> > > + result = -ENOMEM;
> > > + goto error_kmalloc;

You may consider calling it err_pci_disable (or such). This way:
- one can check that the kmalloc is preceeded by a pci_enable;
- one can locally check that the unroll path does its job
(error_pci_disable is followed by a pci_disable_xxx -> makes sense).

--
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:05    [W:0.033 / U:7.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site