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?
Fixes: 5fe8b1cc6a03 ("hwrng: add OP-TEE based rng driver")
Signed-off-by: Wei Yongjun <[email protected]>
---
drivers/char/hw_random/optee-rng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
index 2b9fc8ac5500..3a4dd6fc9ff4 100644
--- a/drivers/char/hw_random/optee-rng.c
+++ b/drivers/char/hw_random/optee-rng.c
@@ -262,7 +262,7 @@ static int optee_rng_remove(struct device *dev)
return 0;
}
-const struct tee_client_device_id optee_rng_id_table[] = {
+static const struct tee_client_device_id optee_rng_id_table[] = {
{UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
{}
On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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?
-Sumit
> Fixes: 5fe8b1cc6a03 ("hwrng: add OP-TEE based rng driver")
> Signed-off-by: Wei Yongjun <[email protected]>
> ---
> drivers/char/hw_random/optee-rng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
> index 2b9fc8ac5500..3a4dd6fc9ff4 100644
> --- a/drivers/char/hw_random/optee-rng.c
> +++ b/drivers/char/hw_random/optee-rng.c
> @@ -262,7 +262,7 @@ static int optee_rng_remove(struct device *dev)
> return 0;
> }
>
> -const struct tee_client_device_id optee_rng_id_table[] = {
> +static const struct tee_client_device_id optee_rng_id_table[] = {
> {UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
> 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
> {}
>
>
>
On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
>
> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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)
> > Fixes: 5fe8b1cc6a03 ("hwrng: add OP-TEE based rng driver")
> > Signed-off-by: Wei Yongjun <[email protected]>
> > ---
> > drivers/char/hw_random/optee-rng.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/hw_random/optee-rng.c b/drivers/char/hw_random/optee-rng.c
> > index 2b9fc8ac5500..3a4dd6fc9ff4 100644
> > --- a/drivers/char/hw_random/optee-rng.c
> > +++ b/drivers/char/hw_random/optee-rng.c
> > @@ -262,7 +262,7 @@ static int optee_rng_remove(struct device *dev)
> > return 0;
> > }
> >
> > -const struct tee_client_device_id optee_rng_id_table[] = {
> > +static const struct tee_client_device_id optee_rng_id_table[] = {
> > {UUID_INIT(0xab7a617c, 0xb8e7, 0x4d8f,
> > 0x83, 0x01, 0xd0, 0x9b, 0x61, 0x03, 0x6b, 0x64)},
> > {}
> >
> >
> >
On Wed, Feb 20, 2019 at 04:04:17PM +0530, Sumit Garg wrote:
> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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?
>
It's from the Sparse tool.
regards,
dan carpenter
On 20/02/2019 10:37, Ard Biesheuvel wrote:
> On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
>>
>> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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)
>
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
On Wed, 20 Feb 2019 at 16:19, Colin Ian King <[email protected]> wrote:
>
> On 20/02/2019 10:37, Ard Biesheuvel wrote:
> > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
> >>
> >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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.
> 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.
-Sumit
On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <[email protected]> wrote:
>
> On Wed, 20 Feb 2019 at 16:19, Colin Ian King <[email protected]> wrote:
> >
> > On 20/02/2019 10:37, Ard Biesheuvel wrote:
> > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
> > >>
> > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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?
> > 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.
I also recommend building with 'make W=1', 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
Arnd
Hi Arnd,
On Wed, 20 Feb 2019 at 21:04, Arnd Bergmann <[email protected]> wrote:
>
> On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <[email protected]> wrote:
> >
> > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <[email protected]> wrote:
> > >
> > > On 20/02/2019 10:37, Ard Biesheuvel wrote:
> > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
> > > >>
> > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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 <[email protected]>
> 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/[email protected]/msg1935826.html
https://www.mail-archive.com/[email protected]/msg1935849.html
One more reported by Dan Carpenter here:
https://www.mail-archive.com/[email protected]/msg1936881.html
Thanks,
Sumit
> Arnd
On Wed, Feb 20, 2019 at 09:55:50PM +0530, Sumit Garg wrote:
> Hi Arnd,
>
> On Wed, 20 Feb 2019 at 21:04, Arnd Bergmann <[email protected]> wrote:
> >
> > On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <[email protected]> wrote:
> > >
> > > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <[email protected]> wrote:
> > > >
> > > > On 20/02/2019 10:37, Ard Biesheuvel wrote:
> > > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
> > > > >>
> > > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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]
Ick, really obsolete, please use the sparse version on kernel.org for
kernel stuff:
git://git.kernel.org/pub/scm/devel/sparse/sparse.git
Loads of things have been fixed and resolved from 0.5.0 which is very
old.
thanks,
greg k-h
On Wed, Feb 20, 2019 at 5:26 PM Sumit Garg <[email protected]> wrote:
> On Wed, 20 Feb 2019 at 21:04, Arnd Bergmann <[email protected]> wrote:
> Reviewed-by: Sumit Garg <[email protected]>
>
> There are already fixes sent in upstream for these. Maybe you could
> pick those too.
>
> https://www.mail-archive.com/[email protected]/msg1935826.html
> https://www.mail-archive.com/[email protected]/msg1935849.html
>
> One more reported by Dan Carpenter here:
>
> https://www.mail-archive.com/[email protected]/msg1936881.html
>
Applied all for now, thanks a lot!
Arnd
On Wed, Feb 20, 2019 at 4:23 AM Wei Yongjun <[email protected]> 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?
Static limits visibility to the current translation unit. Static is
like private visibility.
Maybe you are thinking if it should be declared extern so other
translation units can find the symbol. extern is like public
visibility.
Jeff
On Wed, 20 Feb 2019 at 22:03, Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Wed, Feb 20, 2019 at 09:55:50PM +0530, Sumit Garg wrote:
> > Hi Arnd,
> >
> > On Wed, 20 Feb 2019 at 21:04, Arnd Bergmann <[email protected]> wrote:
> > >
> > > On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <[email protected]> wrote:
> > > >
> > > > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <[email protected]> wrote:
> > > > >
> > > > > On 20/02/2019 10:37, Ard Biesheuvel wrote:
> > > > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <[email protected]> wrote:
> > > > > >>
> > > > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <[email protected]> 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]
>
> Ick, really obsolete, please use the sparse version on kernel.org for
> kernel stuff:
> git://git.kernel.org/pub/scm/devel/sparse/sparse.git
>
> Loads of things have been fixed and resolved from 0.5.0 which is very
> old.
>
Thanks for this info. Now it works pretty well.
-Sumit
> thanks,
>
> greg k-h