lkml.org 
[lkml]   [2020]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 11/30] drm/tegra: dc: Support OPP and SoC core voltage scaling
On Tue, Nov 10, 2020 at 09:29:45PM +0100, Thierry Reding wrote:
> > + err = dev_pm_opp_of_add_table(dc->dev);
> > + if (err) {
> > + dev_err(dc->dev, "failed to add OPP table: %d\n", err);
> > + goto put_hw;
> > + }
> > +
> > + err = devm_add_action(dc->dev, tegra_dc_deinit_opp_table, dc);
> > + if (err)
> > + goto remove_table;
>
> Do these functions return positive values? If not, I'd prefer if this
> check was more explicit (i.e. err < 0) for consistency with the rest of
> this code.
>

Isn't it the other way around? It's only when the check is explicitly
for "if (ret < 0)" that we have to wonder about positives. If the codes
says "if (ret)" then we know that it doesn't return positive values and
every non-zero is an error.

In the kernel "if (ret)" is way more popular than "if (ret < 0)":

$ git grep 'if (\(ret\|rc\|err\))' | wc -l
92927
$ git grep 'if (\(ret\|rc\|err\) < 0)' | wc -l
36577

And some of those are places where "ret" can be positive so we are
forced to use the "if (ret < 0)" format.

Checking for "if (ret)" is easier from a static analysis perspective.
If it's one style is used consistently then they're the same but when
there is a mismatch the "if (ret < 0) " will trigger a false positive
and the "if (ret) " will not.

int var;

ret = frob(&var);
if (ret < 0)
return ret;

Smatch thinks positive returns are not handled so it complains that
"var can be uninitialized".

regards,
dan carpenter

\
 
 \ /
  Last update: 2020-11-11 10:29    [W:0.123 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site