From b1dd11d60e5357c13d3f3decfb69bd07dde159bd Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Thu, 9 May 2013 08:27:25 -0700 Subject: [PATCH] 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 Signed-off-by: Tony Lindgren --- arch/arm/mach-omap2/id.c | 5 ++--- 1 file 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) 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 */ -- 2.20.1