acpi:glue.c Fix warning: variable 'ret' set but not used
in [Kernel]
|
Prev: [PATCH] Adding four read-only files to /proc/sys/vm
Next: bluetooth:hci_bcsp Fix operation on 'bcsp->msgq_txseq' may be undefined
From: Justin P. Mattock on 28 Jun 2010 14:00 On 06/28/2010 05:48 AM, David Howells wrote: > Justin P. Mattock<justinmattock(a)gmail.com> wrote: > >> + if (ret) { >> + printk(KERN_WARNING "dev%d: Failed to get exception\n", ret); >> + } > > That's not a good warning because it's a meaningless string and you're passing > the error number to the dev%d. > > Better would perhaps be: > > "dev%d: Failed to create physical_node sysfs link: %d\n" > I'll throw that in. > Note also that you're only checking the result of one sysfs_create_link(). > You should probably check both. > this is where I get confused with: &acpi_dev->dev.kobj, &dev->kobji it's like a criss cross of code > Also you're introducing a pair of unnecessary braces as there's only one > statement in the if-body. > > David > would just removing ret be good or will things go out of whack because of no ret Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 28 Jun 2010 15:10 no need to have extra cc's on this, so I took some people off since they dont pertain to this area. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 28 Jun 2010 15:10 On 06/28/2010 11:47 AM, David Howells wrote: > Justin P. Mattock<justinmattock(a)gmail.com> wrote: > >> would just removing ret be good or will things go out of whack because of no >> ret > > Don't you then get warnings for not capturing the result? > > David > not sure if it would give warnings for not capturing the result. then doing this would be the way to go. if (ret) { printk(KERN_WARNING "dev%d: Failed to create physical_node sysfs link: %d\n"); } but like you had said unnecessary braces, and only one statement I'll look at this some more too see if I come up with anything. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 28 Jun 2010 23:30 o.k. after stepping out for a while.. I'm finally sitting down and looking at this. below is what I came up with.. hopefully it's in the area/vecinity of what might be a good idea for this(if not then let me know) From da5cfa463f29ff3fe4af3874649db0809e50ab96 Mon Sep 17 00:00:00 2001 From: Justin P. Mattock <justinmattock(a)gmail.com> Date: Mon, 28 Jun 2010 20:14:50 -0700 Subject: [PATCH] glue.c Signed-off-by: Justin P. Mattock <justinmattock(a)gmail.com> --- drivers/acpi/glue.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c index 4af6301..970d7f3 100644 --- a/drivers/acpi/glue.c +++ b/drivers/acpi/glue.c @@ -145,6 +145,7 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle) { struct acpi_device *acpi_dev; acpi_status status; + int fn, pn; if (dev->archdata.acpi_handle) { dev_warn(dev, "Drivers changed 'acpi_handle'\n"); @@ -160,12 +161,17 @@ static int acpi_bind_one(struct device *dev, acpi_handle handle) status = acpi_bus_get_device(handle, &acpi_dev); if (!ACPI_FAILURE(status)) { - int ret; - ret = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj, + fn = sysfs_create_link(&dev->kobj, &acpi_dev->dev.kobj, "firmware_node"); - ret = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj, + pn = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj, "physical_node"); + if (fn) { + printk(KERN_WARNING "dev%d: Failed to create firmware_node: %d\n", status, fn); + }else if (pn) { + printk(KERN_WARNING "dev%d: Failed to create physical_node: %d\n", status, pn); + return 0; + } if (acpi_dev->wakeup.flags.valid) { device_set_wakeup_capable(dev, true); device_set_wakeup_enable(dev, -- 1.7.1.rc1.21.gf3bd6 keep in mind Im not exactly sure what should go into the printk as for words to say, and functions, so I just used status, fn/pn for the two because I was getting a not enough function error. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Justin P. Mattock on 29 Jun 2010 13:20
On 06/29/2010 08:47 AM, David Howells wrote: > Justin P. Mattock<justinmattock(a)gmail.com> wrote: > >> + if (fn) { >> + printk(KERN_WARNING "dev%d: Failed to create >> firmware_node: %d\n", status, fn); >> + }else if (pn) { >> + printk(KERN_WARNING "dev%d: Failed to create >> physical_node: %d\n", status, pn); >> + return 0; >> + } > > The if-statement should be correctly indented (it's inside another > if-body, so needs to be one more tab over) and there needs to be a space > before the else. > o.k., so it should look something like this: if (fn) { code... } else if (pn) { > You should probably split your printks up so they don't exceed 80 chars too, > for example: > > printk(KERN_WARNING > "dev%d: Failed to create physical_node: %d\n", > status, pn); > > Also 'status' is probably the wrong thing to print as the number in "dev%d". > If it worked, that should be unconditionally AE_OK, I think. Can you not use > dev_warn() or similar instead or printk? > > David > alright, I'll look at this today. I'm not the best at making printks in fact I'm more intimidated by them..(so with this in mind, I'm going to sit and make myself learn this, so I atleast have a better idea of doing these than I have now.) Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |