Skip to contentSkip to conversation

checkpatch: false positives for else after return

DISCUSSION Thierry Reding 2 participants 2 messages Jul 18, 2016

lore
Share & tools
lore
#0 lore raw

Hi,

I've run across what I think is a false positive for checkpatch's UNNECESSARY_ELSE check. The code that triggers it is in the tegra_sor_probe() function in drivers/gpu/drm/tegra/sor.c. For reference, here's the code:

	if (sor->soc->supports_hdmi) {
		sor->ops = &tegra_sor_hdmi_ops;
	} else if (sor->soc->supports_lvds) {
		dev_err(&pdev->dev, "LVDS not supported yet\n");
		return -ENODEV;
	} else {
		dev_err(&pdev->dev, "unknown (non-DP) support\n");
		return -ENODEV;
	}

For the first case (HDMI supported on SoC) the code should continue normally, but otherwise we need to error out because we don't support the configuration.

I can't come up with an alternative way of writing the above, and at the same time I can't see what's wrong with the above. It looks like a legitimate use of an else to me.

I made an attempt at fixing the check myself but failed miserably. Regex isn't among my strong skills =\

Any ideas on how to deal with this?

Thanks,
Thierry

#1 lore raw
Quoted 21 lines · Thierry Reding

Hi,

I've run across what I think is a false positive for checkpatch's UNNECESSARY_ELSE check. The code that triggers it is in the tegra_sor_probe() function in drivers/gpu/drm/tegra/sor.c. For reference, here's the code:

	if (sor->soc->supports_hdmi) {
		sor->ops = &tegra_sor_hdmi_ops;
	} else if (sor->soc->supports_lvds) {
		dev_err(&pdev->dev, "LVDS not supported yet\n");
		return -ENODEV;
	} else {
		dev_err(&pdev->dev, "unknown (non-DP) support\n");
		return -ENODEV;
	}

For the first case (HDMI supported on SoC) the code should continue normally, but otherwise we need to error out because we don't support the configuration.

Quoted 8 lines

I can't come up with an alternative way of writing the above, and at the same time I can't see what's wrong with the above. It looks like a legitimate use of an else to me.

I made an attempt at fixing the check myself but failed miserably. Regex isn't among my strong skills =\

Any ideas on how to deal with this?

Hi Thierry.

Ignore checkpatch when it's wrong.

The message is:
"else is not generally useful after a break or return" and that statement is true.

checkpatch is not, and will not become, a code flow analysis tool.  It's a very brain-dead, stupid little script that looks at very simple patch table rules.