Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1445400 > unrolled thread

checkpatch: false positives for else after return

Back to article view | Back to linux.kernel

Contents

  checkpatch: false positives for else after return Thierry Reding <thierry.reding@gmail.com> - 2016-07-18 13:30 +0200
    Re: checkpatch: false positives for else after return Joe Perches <joe@perches.com> - 2016-07-18 17:40 +0200

#1445400 — checkpatch: false positives for else after return

[Multipart message — attachments visible in raw view] — view 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

[toc] | [next] | [standalone]

#1445593

On Mon, 2016-07-18 at 13:26 +0200, Thierry Reding wrote:
> 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?

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web