login
Search
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>
---
drivers/pci/probe.c: In function `pci_create_bus':
drivers/pci/probe.c:66: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result
drivers/pci/probe.c:74: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result
# ia64-unknown-linux-gnu-gcc --version
ia64-unknown-linux-gnu-gcc (GCC) 3.4.5
Copyright (C) 2004 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
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 = ...Hi Horms, Fixing these kinds of warnings is highly useful but thankless work, so I =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= And here? cheers --=20 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
Here too. Reason: If we fail to create the legacy_io file, legacy_mem will still be NULL, because it has been not initialized at that point. But we will try to remove it in pci_remove_legacy_files and in sysfs_remove_bin_file --
Check the return value of device_create_bin_file in pci_create_bus,
unwind if necessary, and propagate any errors to the caller.
Cc: Sven Wegener <sven.wegener@stealer.net>
Cc: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
Revised the patch to free and NULLify b->legacy_io in
pci_create_legacy_files() on error. Thanks to
Sven Wegener and Michael Ellerman for pointing out this omission.
This patch resolves the following warnings:
drivers/pci/probe.c: In function `pci_create_bus':
drivers/pci/probe.c:66: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result
drivers/pci/probe.c:74: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result
# ia64-unknown-linux-gnu-gcc --version
ia64-unknown-linux-gnu-gcc (GCC) 3.4.5
Copyright (C) 2004 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Index: linux-2.6/drivers/pci/probe.c
===================================================================
--- linux-2.6.orig/drivers/pci/probe.c 2008-08-05 20:50:10.000000000 +1000
+++ linux-2.6/drivers/pci/probe.c 2008-08-05 21:08:27.000000000 +1000
@@ -53,26 +53,42 @@ 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, ...Yes, but you're essentially saying here that if I can't create a couple of poxy sysfs files, I can't have this PCI bus at all? This seems like a bad decision to me. I'd rather have a PCI bus without the files than no PCI bus at all. By all means, we should whinge mightily if we can't create the files so the sysadmin has a chance of figuring out why things aren't quite working right, but I might have my root filesystem on a device on that PCI bus. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." --
Are you suggesting just making pci_create_bus() have a big winge using printk() but returning void regardless of what happens? That sounds fine to me, though I guess it would also need to unwind if the first call to device_create_bin_file() succeeds but the second one doesn't. --
Sounds like the right course of action to me. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." --
Check the return value of device_create_bin_file in pci_create_bus and unwind if necessary. Don't propagate error to caller, as failure to create these files shouldn't prevent PCI from being initialised. Cc: Sven Wegener <sven.wegener@stealer.net> Cc: Michael Ellerman <michael@ellerman.id.au> Cc: Matthew Wilcox <matthew@wil.cx> Signed-off-by: Simon Horman <horms@verge.net.au> --- Tue, 5 Aug 2008 21:14:07 +1000 * Revised the patch to free and NULLify b->legacy_io in pci_create_legacy_files() on error. Thanks to Sven Wegener and Michael Ellerman for pointing out this omission. Wed, 06 Aug 2008 10:49:30 +1000 * Don't propagate error to caller, as failure to create these files shouldn't prevent PCI from being initialised. Thanks to Matthew Wilcox This patch resolves the following warnings: drivers/pci/probe.c: In function `pci_create_bus': drivers/pci/probe.c:66: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result drivers/pci/probe.c:74: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result # ia64-unknown-linux-gnu-gcc --version ia64-unknown-linux-gnu-gcc (GCC) 3.4.5 Copyright (C) 2004 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Index: linux-2.6/drivers/pci/probe.c =================================================================== --- linux-2.6.orig/drivers/pci/probe.c 2008-08-06 09:22:06.000000000 +1000 +++ linux-2.6/drivers/pci/probe.c 2008-08-06 09:41:09.000000000 +1000 @@ -52,27 +52,46 @@ EXPORT_SYMBOL(no_pci_devices); * Some platforms allow access to legacy I/O port and ISA memory space on * a per-bus basis. This routine creates the files and ties them into * their associated read, write and mmap files from pci-sysfs.c + * + * On error unwind, but don't propogate the error to the caller + * as it is ok to ...
I'm sure the compiler warns about that. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." --
Amusingly, yes. -- Horms -> looks for somewhere to hide --
Check the return value of device_create_bin_file in pci_create_bus and unwind if necessary. Don't propagate error to caller, as failure to create these files shouldn't prevent PCI from being initialised. Cc: Sven Wegener <sven.wegener@stealer.net> Cc: Michael Ellerman <michael@ellerman.id.au> Cc: Matthew Wilcox <matthew@wil.cx> Signed-off-by: Simon Horman <horms@verge.net.au> --- Tue, 5 Aug 2008 21:14:07 +1000 * Revised the patch to free and NULLify b->legacy_io in pci_create_legacy_files() on error. Thanks to Sven Wegener and Michael Ellerman for pointing out this omission. Wed, 06 Aug 2008 10:49:30 +1000 * Don't propagate error to caller, as failure to create these files shouldn't prevent PCI from being initialised. Thanks to Matthew Wilcox Thu, 07 Aug 2008 09:28:56 +1000 * Remove spurious return value. Amusingly this patch was introducing warnings :-) This patch resolves the following warnings: drivers/pci/probe.c: In function `pci_create_bus': drivers/pci/probe.c:66: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result drivers/pci/probe.c:74: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result # ia64-unknown-linux-gnu-gcc --version ia64-unknown-linux-gnu-gcc (GCC) 3.4.5 Copyright (C) 2004 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Index: linux-2.6/drivers/pci/probe.c =================================================================== --- linux-2.6.orig/drivers/pci/probe.c 2008-08-07 09:08:32.000000000 +1000 +++ linux-2.6/drivers/pci/probe.c 2008-08-07 09:14:53.000000000 +1000 @@ -52,27 +52,46 @@ EXPORT_SYMBOL(no_pci_devices); * Some platforms allow access to legacy I/O port and ISA memory space on * a per-bus basis. This routine creates the files and ties them into * their associated read, write and ...
I hate to prolong the pain, but wasn't the plan to print a big fat warning so that it's clear the system is not 100% happy. cheers --=20 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
Check the return value of device_create_bin_file in pci_create_bus and unwind if necessary. Don't propagate error to caller, as failure to create these files shouldn't prevent PCI from being initialised. Instead, just log a warning. Cc: Sven Wegener <sven.wegener@stealer.net> Cc: Michael Ellerman <michael@ellerman.id.au> Cc: Matthew Wilcox <matthew@wil.cx> Signed-off-by: Simon Horman <horms@verge.net.au> --- Tue, 5 Aug 2008 21:14:07 +1000 * Revised the patch to free and NULLify b->legacy_io in pci_create_legacy_files() on error. Wed, 06 Aug 2008 10:49:30 +1000 * Don't propagate error to caller, as failure to create these files shouldn't prevent PCI from being initialised. Thu, 07 Aug 2008 09:28:56 +1000 * Remove spurious return value. Amusingly this patch was introducing warnings :-) Thu, 07 Aug 2008 11:15:32 +1000 * Print a warning if legacy file initialisation fails. This patch resolves the following warnings: drivers/pci/probe.c: In function `pci_create_bus': drivers/pci/probe.c:66: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result drivers/pci/probe.c:74: warning: ignoring return value of `device_create_bin_file', declared with attribute warn_unused_result # ia64-unknown-linux-gnu-gcc --version ia64-unknown-linux-gnu-gcc (GCC) 3.4.5 Copyright (C) 2004 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Index: linux-2.6/drivers/pci/probe.c =================================================================== --- linux-2.6.orig/drivers/pci/probe.c 2008-08-07 09:08:32.000000000 +1000 +++ linux-2.6/drivers/pci/probe.c 2008-08-07 11:21:31.000000000 +1000 @@ -52,27 +52,49 @@ EXPORT_SYMBOL(no_pci_devices); * Some platforms allow access to legacy I/O port and ISA memory space on * a per-bus basis. This routine creates the files and ties them into * their associated ...
Applied, thanks for your persistence, Simon! :) Thanks, Jesse --
Thanks, sorry for not getting it right much earlier. --
Uhm, this case can not happen in this version of the patch. We'll actually fail registering the bus completely with this patch, so there should be no chance that we'll ever go through pci_remove_legacy_files with legacy_io != NULL and legacy_mem == NULL. So no need to NULLify it. But with the change Matthew suggested, we need to pay attention to it. Sorry for the --
