diff options
author | Tony Lindgren <tony@atomide.com> | 2013-05-09 11:27:25 -0400 |
---|---|---|
committer | Tony Lindgren <tony@atomide.com> | 2013-05-09 12:06:27 -0400 |
commit | b1dd11d60e5357c13d3f3decfb69bd07dde159bd (patch) | |
tree | 665f023a7ead23abfeac88f6c842323783be92b9 | |
parent | 827897c05c3da95dcd0ae44549563525a3554e00 (diff) |
ARM: OMAP2+: Remove bogus IS_ERR_OR_NULL checking from id.c
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>
-rw-r--r-- | arch/arm/mach-omap2/id.c | 5 |
1 files changed, 2 insertions, 3 deletions
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c index 9bc5a18794f9..1272c41d4749 100644 --- a/arch/arm/mach-omap2/id.c +++ b/arch/arm/mach-omap2/id.c | |||
@@ -648,13 +648,12 @@ void __init omap_soc_device_init(void) | |||
648 | soc_dev_attr->revision = soc_rev; | 648 | soc_dev_attr->revision = soc_rev; |
649 | 649 | ||
650 | soc_dev = soc_device_register(soc_dev_attr); | 650 | soc_dev = soc_device_register(soc_dev_attr); |
651 | if (IS_ERR_OR_NULL(soc_dev)) { | 651 | if (IS_ERR(soc_dev)) { |
652 | kfree(soc_dev_attr); | 652 | kfree(soc_dev_attr); |
653 | return; | 653 | return; |
654 | } | 654 | } |
655 | 655 | ||
656 | parent = soc_device_to_device(soc_dev); | 656 | parent = soc_device_to_device(soc_dev); |
657 | if (!IS_ERR_OR_NULL(parent)) | 657 | device_create_file(parent, &omap_soc_attr); |
658 | device_create_file(parent, &omap_soc_attr); | ||
659 | } | 658 | } |
660 | #endif /* CONFIG_SOC_BUS */ | 659 | #endif /* CONFIG_SOC_BUS */ |