Hi Arnd,
On Wed, 20 Feb 2019 at 21:04, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
> >
> > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <colin.king@xxxxxxxxxxxxx> wrote:
> > >
> > > On 20/02/2019 10:37, Ard Biesheuvel wrote:
> > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
> > > >>
> > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@xxxxxxxxxx> wrote:
> > > >>>
> > > >>> Fixes the following sparse warning:
> > > >>>
> > > >>> drivers/char/hw_random/optee-rng.c:265:35: warning:
> > > >>> symbol 'optee_rng_id_table' was not declared. Should it be static?
> > > >>>
> > > >>
> > > >> I haven't observed this warning during my normal Linux build using
> > > >> gcc. Is there any specific configuration you are using?
> > > >>
> > > >
> > > > This is a sparse warning, not GCC. You need to install it separately
> > > > and build with C=1 (iirc)
> > > >
> >
> > TBH, I wasn't aware about this sparse tool. I did install sparse and
> > build with C=1 option. But I could only get following such
> > errors/warnings for drivers/char/hw_random/optee-rng.c:
> >
> > ./arch/arm64/include/asm/lse.h:18:37: warning: Unknown escape 'l'
> > ./arch/arm64/include/asm/alternative.h:213:28: warning: Unknown escape 'o'
> > ./include/linux/compiler.h:194:8: error: attribute '__gnu_inline__':
> > unknown attribute
> >
> > But not the one mentioned in this patch.
>
> Not sure what went wrong, I see the same warning as the others.
> Maybe you have an outdated version of sparse that runs into unrelated
> issues?
>
$ apt list sparse
Listing... Done
sparse/xenial,now 0.5.0-1build1 amd64 [installed]
> > > It's useful to may these symbols static just to reduce the scope and
> > > there is on-going work to fix these symbols up across the entire kernel
> > >
> >
> > Agree with this patch to make optee_rng_id_table symbol static.
> > Actually I was curious to know the approach used to get these type of
> > warnings so that they could be fixed beforehand.
>
> If you provide an 'Acked-by' or 'Reviewed-by' tag, I can apply this one
> on top of the other two fixes I already took.
>
Sure.
Reviewed-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
> I also recommend building with 'make W=1',
Thanks for your suggestion.
> which enables additional
> warnings and found this bug in your driver:
>
> drivers/char/hw_random/optee-rng.c: In function 'get_optee_rng_data':
> drivers/char/hw_random/optee-rng.c:94:11: error: comparison of
> unsigned expression < 0 is always false [-Werror=type-limits]
> if ((ret < 0) || (inv_arg.ret != 0)) {
> ^
> drivers/char/hw_random/optee-rng.c: In function 'get_optee_rng_info':
> drivers/char/hw_random/optee-rng.c:188:11: error: comparison of
> unsigned expression < 0 is always false [-Werror=type-limits]
> if ((ret < 0) || (inv_arg.ret != 0)) {
> ^
>
> Here, you probably need to make 'ret' an 'int', and you should drop
> the extraneous zero-initialization for a couple of variables like that one,
> so gcc is able to do its job of warning about uninitialized variable
>
There are already fixes sent in upstream for these. Maybe you could
pick those too.
https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1935826.html
https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1935849.html
One more reported by Dan Carpenter here:
https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1936881.html
Thanks,
Sumit
> Arnd
