* checkpatch: false positives for else after return
@ 2016-07-18 11:26 Thierry Reding
2016-07-18 15:30 ` Joe Perches
0 siblings, 1 reply; 2+ messages in thread
From: Thierry Reding @ 2016-07-18 11:26 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 973 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: checkpatch: false positives for else after return
2016-07-18 11:26 checkpatch: false positives for else after return Thierry Reding
@ 2016-07-18 15:30 ` Joe Perches
0 siblings, 0 replies; 2+ messages in thread
From: Joe Perches @ 2016-07-18 15:30 UTC (permalink / raw)
To: Thierry Reding, Andy Whitcroft; +Cc: linux-kernel
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-07-18 15:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-18 11:26 checkpatch: false positives for else after return Thierry Reding
2016-07-18 15:30 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).