Re: [PATCH] mmc: sdhci-of-arasan: Remove uninitialized ret variables
From: Nathan Chancellor
Date: Thu Apr 16 2020 - 16:32:44 EST
On Thu, Apr 16, 2020 at 01:16:27PM -0700, Nick Desaulniers wrote:
> On Thu, Apr 16, 2020 at 11:24 AM Nathan Chancellor
> <natechancellor@xxxxxxxxx> wrote:
> >
> > Clang warns:
> >
> > drivers/mmc/host/sdhci-of-arasan.c:784:9: warning: variable 'ret' is
> > uninitialized when used here [-Wuninitialized]
> > return ret;
> > ^~~
> > drivers/mmc/host/sdhci-of-arasan.c:738:9: note: initialize the variable
> > 'ret' to silence this warning
> > int ret;
> > ^
> > = 0
> > drivers/mmc/host/sdhci-of-arasan.c:860:9: warning: variable 'ret' is
> > uninitialized when used here [-Wuninitialized]
> > return ret;
> > ^~~
> > drivers/mmc/host/sdhci-of-arasan.c:810:9: note: initialize the variable
> > 'ret' to silence this warning
> > int ret;
> > ^
> > = 0
> > 2 warnings generated.
> >
> > This looks like a copy paste error. Neither function has handling that
> > needs ret so just remove it and return 0 directly.
>
> Forgive me for not taking the time to look into this more carefully,
> but just a thought:
>
> Having functions always return a single integer literal as opposed to
> having a `void` return type in their function signature is a code
> smell. Did you consider the call sites of these functions to see if
> they do anything with the return value? I understand it may not be
> worthwhile/possible if these functions fulfil an interface that
> requires the int return type function signature. (It's also probably
Which is the case. These functions are passed to 'struct clk_ops', which
defines the set_phase member as
int (*set_phase)(struct clk_hw *hw, int degrees);
so we cannot just change the return to void since there are other
set_phase functions that do set a return value other than zero.
> faster for me to just look rather than type this all out, but I saw no
> mention of this consideration in the commit message or patch, so
> wanted to check that it had been performed).
Yeah, I should have probably mentioned that. I can do so if the
maintainers feel it worthwhile.
Cheers,
Nathan