lkml.org 
[lkml]   [2010]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/5]acpi:glue.c Fix warning: variable 'ret' set but not used
On 07/01/2010 02:31 AM, David Howells wrote:
> Justin P. Mattock<justinmattock@gmail.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);

ah... I did think about that a few days ago, but had no idea how to
really follow through with this.. and from looking at what you did, it's
as simple as a || b
>
> 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);

I don't mind resending with your change to this.

>
> Is it that important to know which failed to be created, or that both failed
> to be created?
>
> David
>

maybe a simple test(prog) case can be created to simulate what this is
doing, just to make sure.

Justin P. Mattock


\
 
 \ /
  Last update: 2010-07-01 15:43    [from the cache]
©2003-2011 Jasper Spaans