On 07/01/2010 02:31 AM, David Howells wrote:
Justin P. Mattock<justinmattock@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
