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    [W:0.025 / U:62.484 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site