[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <16513.1277976706@redhat.com>
Date: Thu, 01 Jul 2010 10:31:46 +0100
From: David Howells <dhowells@...hat.com>
To: "Justin P. Mattock" <justinmattock@...il.com>
Cc: dhowells@...hat.com, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, lenb@...nel.org
Subject: Re: [PATCH 3/5]acpi:glue.c Fix warning: variable 'ret' set but not used
Justin P. Mattock <justinmattock@...il.com> wrote:
> + if (fn) {
> + dev_warn(&acpi_dev->dev,
> + "Failed to create firmware_node link to %s %s: %d\n",
> + dev_driver_string(dev), dev_name(dev), fn);
> + } else if (pn) {
> + dev_warn(&acpi_dev->dev,
> + "Failed to create physical_node link to %s %s: %d\n",
> + dev_driver_string(dev), dev_name(dev), pn);
> + return AE_ERROR;
> + }
There's one more question to ask yourself: do you really need two dev_warn()
statements? You could have just one that prints both error values:
if (fn || pn)
dev_warn(&acpi_dev->dev,
"Failed to create link(s) to %s %s:"
" fn=%d pn=%d\n",
dev_driver_string(dev), dev_name(dev),
fn, pn);
Not sure it's worth going that far. You could reduce it still further:
if (fn || pn)
dev_warn(&acpi_dev->dev,
"Failed to create link(s) to %s %s:"
" %d\n",
dev_driver_string(dev), dev_name(dev),
fn ?: pn);
Is it that important to know which failed to be created, or that both failed
to be created?
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists