Arnd Bergmann
2017-Sep-06 13:56 UTC
[Nouveau] [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
gcc thinks that interpreting a multiplication result as a bool is confusing: drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll': drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context] In this instance, I think using multiplication is more intuitive than '&&', so I'm adding a comparison to zero instead to shut up the warning. To further improve readability, I also make the error case indented and leave the normal case as the final 'return' statement. Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary change)") Signed-off-by: Arnd Bergmann <arnd at arndb.de> --- Originally submitted on July 14, but no reply. This is the same patch again. The warning is currently disabled in mainline, but I think we can turn it back on in the future, and this change here seems harmless. --- drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c index 96e0941c8edd..04b4f4ccf186 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c @@ -130,10 +130,10 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll) sclk = read_clk(clk, 0x10 + idx, false); } - if (M * P) - return sclk * N / (M * P); + if (M * P == 0) + return 0; - return 0; + return sclk * N / (M * P); } static int -- 2.9.0
Karol Herbst
2017-Sep-06 14:20 UTC
[Nouveau] [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
On Wed, Sep 6, 2017 at 3:56 PM, Arnd Bergmann <arnd at arndb.de> wrote:> gcc thinks that interpreting a multiplication result as a bool > is confusing: > > drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c: In function 'read_pll': > drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c:133:8: error: '*' in boolean context, suggest '&&' instead [-Werror=int-in-bool-context] > > In this instance, I think using multiplication is more intuitive > than '&&', so I'm adding a comparison to zero instead to shut up > the warning. To further improve readability, I also make the > error case indented and leave the normal case as the final 'return' > statement. >I think to make perfectly clear why this check is done, we simply should precompute the denominator and do something like this: int denominator = M * P if (denominator == 0) return 0; return sclk * N / denominator; but with a better name for "denominator". I even imagine, that there are some macros in the kernel for dealing with something like this already, so that we could do instead: return AWESOME_DIV_MACRO(sclk * N, M * P)> Fixes: 7632b30e4b8b ("drm/nouveau/clk: namespace + nvidia gpu names (no binary change)") > Signed-off-by: Arnd Bergmann <arnd at arndb.de> > --- > Originally submitted on July 14, but no reply. This is the same > patch again. The warning is currently disabled in mainline, but > I think we can turn it back on in the future, and this change here > seems harmless. > --- > drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c > index 96e0941c8edd..04b4f4ccf186 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/gt215.c > @@ -130,10 +130,10 @@ read_pll(struct gt215_clk *clk, int idx, u32 pll) > sclk = read_clk(clk, 0x10 + idx, false); > } > > - if (M * P) > - return sclk * N / (M * P); > + if (M * P == 0) > + return 0; > > - return 0; > + return sclk * N / (M * P); > } > > static int > -- > 2.9.0 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
Arnd Bergmann
2017-Sep-06 20:11 UTC
[Nouveau] [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
On Wed, Sep 6, 2017 at 4:20 PM, Karol Herbst <karolherbst at gmail.com> wrote:>> In this instance, I think using multiplication is more intuitive >> than '&&', so I'm adding a comparison to zero instead to shut up >> the warning. To further improve readability, I also make the >> error case indented and leave the normal case as the final 'return' >> statement. >> > > I think to make perfectly clear why this check is done, we simply > should precompute the denominator and do something like this: > > int denominator = M * P > if (denominator == 0) > return 0; > return sclk * N / denominator; > > but with a better name for "denominator".I don't know what M and P actually are in this function, so I couldn't come up with a much better name either, how about simply 'divisor'?> I even imagine, that there > are some macros in the kernel for dealing with something like this > already, so that we could do instead: > > return AWESOME_DIV_MACRO(sclk * N, M * P)I don't think that exists, the behavior for divide-by-zero really depends a lot on the context, and returning zero is probably often not a good solution. Arnd
Seemingly Similar Threads
- [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
- [PATCH 12/14] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
- [PATCH] [v2] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
- [PATCH] [RESEND v2] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning
- [PATCH] [RESEND] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning