[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <YhT/73ZM7IytO3+Q@dev-arch.archlinux-ax161>
Date: Tue, 22 Feb 2022 08:23:27 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Philippe CORNU <philippe.cornu@...s.st.com>
Cc: Nick Desaulniers <ndesaulniers@...gle.com>,
Yannick Fertre <yannick.fertre@...s.st.com>,
Benjamin Gaignard <benjamin.gaignard@...aro.org>,
dri-devel@...ts.freedesktop.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
llvm@...ts.linux.dev
Subject: Re: [PATCH] drm/stm: Avoid using val uninitialized in
ltdc_set_ycbcr_config()
On Tue, Feb 22, 2022 at 11:54:04AM +0100, Philippe CORNU wrote:
>
>
> On 2/7/22 8:44 PM, Nick Desaulniers wrote:
> > On Mon, Feb 7, 2022 at 8:53 AM Nathan Chancellor <nathan@...nel.org> wrote:
> > >
> > > Clang warns:
> > >
> > > drivers/gpu/drm/stm/ltdc.c:625:2: warning: variable 'val' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
> > > default:
> > > ^~~~~~~
> > > drivers/gpu/drm/stm/ltdc.c:635:2: note: uninitialized use occurs here
> > > val |= LxPCR_YCEN;
> > > ^~~
> > > drivers/gpu/drm/stm/ltdc.c:600:9: note: initialize the variable 'val' to silence this warning
> > > u32 val;
> > > ^
> > > = 0
> > > 1 warning generated.
> > >
> > > Use a return instead of break in the default case to fix the warning.
> > > Add an error message so that this return is not silent, which could hide
> > > issues in the future.
> > >
> > > Fixes: 484e72d3146b ("drm/stm: ltdc: add support of ycbcr pixel formats")
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1575
> > > Signed-off-by: Nathan Chancellor <nathan@...nel.org>
> > > ---
> > > drivers/gpu/drm/stm/ltdc.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> > > index 5eeb32c9c9ce..447ddde1786c 100644
> > > --- a/drivers/gpu/drm/stm/ltdc.c
> > > +++ b/drivers/gpu/drm/stm/ltdc.c
> > > @@ -624,7 +624,8 @@ static inline void ltdc_set_ycbcr_config(struct drm_plane *plane, u32 drm_pix_fm
> > > break;
> > > default:
> > > /* RGB or not a YCbCr supported format */
> > > - break;
> > > + drm_err(plane->dev, "Unsupported pixel format: %u\n", drm_pix_fmt);
> >
> > This is fine, but in the future you should add an explicit
> > #include <drm/drm_print.h>
> > to avoid implicit header dependencies (like the ones that Mingo is
> > trying to detangle) for the declaration of drm_err. `drm_vprintf`
> > needs it, too.
> >
> > Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>
> >
>
> Hi Nick,
> and thank you for having pointing this.
>
> Hi Nathan,
> May I ask you please to update your patch changing drm_err(plane->dev, )
> with DRM_ERROR().
Sure thing, v2 has been sent:
https://lore.kernel.org/r/20220222152045.484610-1-nathan@kernel.org/
I used drm_err() as I saw DRM_ERROR() was deprecated but I get internal
driver consistency is important.
Cheers,
Nathan
> Big thank you,
>
> Philippe :-)
>
>
>
> > > + return;
> > > }
> > >
> > > /* Enable limited range */
> > >
> > > base-commit: 542898c5aa5c6a3179dffb1d1606884a63f75fed
> > > --
> > > 2.35.1
> > >
> >
> >
Powered by blists - more mailing lists