lkml.org 
[lkml]   [2008]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [patch] PCI: check the return value of device_create_bin_file() in pci_create_bus()
    From
    Date
    On Tue, 2008-08-05 at 20:16 +1000, Simon Horman wrote:
    > Check the return value of device_create_bin_file in pci_create_bus,
    > unwind if neccessary, and propogate any errors to the caller.
    >
    > Signed-off-by: Simon Horman <horms@verge.net.au>

    Hi Horms,

    Fixing these kinds of warnings is highly useful but thankless work, so I
    hate to nit pick ..

    > Index: linux-2.6/drivers/pci/probe.c
    > ===================================================================
    > --- linux-2.6.orig/drivers/pci/probe.c 2008-08-05 19:58:43.000000000 +1000
    > +++ linux-2.6/drivers/pci/probe.c 2008-08-05 19:59:15.000000000 +1000
    > @@ -53,26 +53,37 @@ EXPORT_SYMBOL(no_pci_devices);
    > * a per-bus basis. This routine creates the files and ties them into
    > * their associated read, write and mmap files from pci-sysfs.c
    > */
    > -static void pci_create_legacy_files(struct pci_bus *b)
    > +static int pci_create_legacy_files(struct pci_bus *b)
    > {
    > + int error;
    > +
    > b->legacy_io = kzalloc(sizeof(struct bin_attribute) * 2,
    > GFP_ATOMIC);
    > - if (b->legacy_io) {
    > - b->legacy_io->attr.name = "legacy_io";
    > - b->legacy_io->size = 0xffff;
    > - b->legacy_io->attr.mode = S_IRUSR | S_IWUSR;
    > - b->legacy_io->read = pci_read_legacy_io;
    > - b->legacy_io->write = pci_write_legacy_io;
    > - device_create_bin_file(&b->dev, b->legacy_io);
    > -
    > - /* Allocated above after the legacy_io struct */
    > - b->legacy_mem = b->legacy_io + 1;
    > - b->legacy_mem->attr.name = "legacy_mem";
    > - b->legacy_mem->size = 1024*1024;
    > - b->legacy_mem->attr.mode = S_IRUSR | S_IWUSR;
    > - b->legacy_mem->mmap = pci_mmap_legacy_mem;
    > - device_create_bin_file(&b->dev, b->legacy_mem);
    > + if (!b->legacy_io)
    > + return -ENOMEM;
    > +
    > + b->legacy_io->attr.name = "legacy_io";
    > + b->legacy_io->size = 0xffff;
    > + b->legacy_io->attr.mode = S_IRUSR | S_IWUSR;
    > + b->legacy_io->read = pci_read_legacy_io;
    > + b->legacy_io->write = pci_write_legacy_io;
    > + error = device_create_bin_file(&b->dev, b->legacy_io);
    > + if (error)
    > + return error;

    Should we kfree(b->legacy_io) here?

    > +
    > + /* Allocated above after the legacy_io struct */
    > + b->legacy_mem = b->legacy_io + 1;
    > + b->legacy_mem->attr.name = "legacy_mem";
    > + b->legacy_mem->size = 1024*1024;
    > + b->legacy_mem->attr.mode = S_IRUSR | S_IWUSR;
    > + b->legacy_mem->mmap = pci_mmap_legacy_mem;
    > + error = device_create_bin_file(&b->dev, b->legacy_mem);
    > + if (error) {
    > + device_remove_bin_file(&b->dev, b->legacy_io);
    > + return error;

    And here?

    cheers

    --
    Michael Ellerman
    OzLabs, IBM Australia Development Lab

    wwweb: http://michael.ellerman.id.au
    phone: +61 2 6212 1183 (tie line 70 21183)

    We do not inherit the earth from our ancestors,
    we borrow it from our children. - S.M.A.R.T Person
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2008-08-05 12:35    [W:0.023 / U:17.128 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site