ARM: OMAP2+: Remove bogus IS_ERR_OR_NULL checking from id.c
authorTony Lindgren <tony@atomide.com>
Thu, 9 May 2013 15:27:25 +0000 (08:27 -0700)
committerTony Lindgren <tony@atomide.com>
Thu, 9 May 2013 16:06:27 +0000 (09:06 -0700)
Commit 6770b211 (ARM: OMAP2+: Export SoC information to userspace)
had some broken return value handling as noted by Russell King:

+       soc_dev = soc_device_register(soc_dev_attr);
+       if (IS_ERR_OR_NULL(soc_dev)) {
+               kfree(soc_dev_attr);
+               return;
+       }
+
+       parent = soc_device_to_device(soc_dev);
+       if (!IS_ERR_OR_NULL(parent))
+               device_create_file(parent, &omap_soc_attr);

This is nonsense.  For the first, IS_ERR() is sufficient.  For the second,
tell me what error checking is required in the return value of this
function:

struct device *soc_device_to_device(struct soc_device *soc_dev)
{
        return &soc_dev->dev;
}

when you've already determined that the passed soc_dev is a valid pointer.
If you read the comments against the prototype:

/**
 * soc_device_to_device - helper function to fetch struct device
 * @soc: Previously registered SoC device container
 */
struct device *soc_device_to_device(struct soc_device *soc);

if "soc" is valid, it means the "previously registered SoC device container"
must have succeeded and that can only happen if the struct device has been
registered.  Ergo, there will always be a valid struct device pointer for
any registered SoC device container.  Therefore, if soc_device_register()
succeeds, then the return value from soc_device_to_device() will always be
valid and no error checking of it is required.

Simples.  The rule as ever applies here: get to know the APIs your using
and don't fumble around in the dark hoping that you'll get this stuff
right.

Fix it as noted by Russell.

Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Tony Lindgren <tony@atomide.com>
arch/arm/mach-omap2/id.c

index 9bc5a18794f9acb35c40d0c893535144a6afd50a..1272c41d474903c638e60902e66144e6a3692e79 100644 (file)
@@ -648,13 +648,12 @@ void __init omap_soc_device_init(void)
        soc_dev_attr->revision = soc_rev;
 
        soc_dev = soc_device_register(soc_dev_attr);
-       if (IS_ERR_OR_NULL(soc_dev)) {
+       if (IS_ERR(soc_dev)) {
                kfree(soc_dev_attr);
                return;
        }
 
        parent = soc_device_to_device(soc_dev);
-       if (!IS_ERR_OR_NULL(parent))
-               device_create_file(parent, &omap_soc_attr);
+       device_create_file(parent, &omap_soc_attr);
 }
 #endif /* CONFIG_SOC_BUS */