| Message ID | 20190220093458.159208-1-weiyongjun1@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [-next] hwrng: make symbol 'optee_rng_id_table' static | expand |
On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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 <weiyongjun1@huawei.com> > --- > 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 <sumit.garg@linaro.org> wrote: > > On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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 <weiyongjun1@huawei.com> > > --- > > 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 <weiyongjun1@huawei.com> 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 <sumit.garg@linaro.org> wrote: >> >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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 <colin.king@canonical.com> wrote: > > On 20/02/2019 10:37, Ard Biesheuvel wrote: > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <sumit.garg@linaro.org> wrote: > >> > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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 <sumit.garg@linaro.org> wrote: > > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <colin.king@canonical.com> wrote: > > > > On 20/02/2019 10:37, Ard Biesheuvel wrote: > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <sumit.garg@linaro.org> wrote: > > >> > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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 <arnd@arndb.de> wrote: > > On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <sumit.garg@linaro.org> wrote: > > > > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <colin.king@canonical.com> wrote: > > > > > > On 20/02/2019 10:37, Ard Biesheuvel wrote: > > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <sumit.garg@linaro.org> wrote: > > > >> > > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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@linaro.org> > 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@vger.kernel.org/msg1935826.html https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1935849.html One more reported by Dan Carpenter here: https://www.mail-archive.com/linux-kernel@vger.kernel.org/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 <arnd@arndb.de> wrote: > > > > On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <sumit.garg@linaro.org> wrote: > > > > > > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <colin.king@canonical.com> wrote: > > > > > > > > On 20/02/2019 10:37, Ard Biesheuvel wrote: > > > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <sumit.garg@linaro.org> wrote: > > > > >> > > > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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 <sumit.garg@linaro.org> wrote: > On Wed, 20 Feb 2019 at 21:04, Arnd Bergmann <arnd@arndb.de> wrote: > Reviewed-by: Sumit Garg <sumit.garg@linaro.org> > > There are already fixes sent in upstream for these. Maybe you could > pick those too. > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1935826.html > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1935849.html > > One more reported by Dan Carpenter here: > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1936881.html > Applied all for now, thanks a lot! Arnd
On Wed, Feb 20, 2019 at 4:23 AM Wei Yongjun <weiyongjun1@huawei.com> 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 <gregkh@linuxfoundation.org> 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 <arnd@arndb.de> wrote: > > > > > > On Wed, Feb 20, 2019 at 12:17 PM Sumit Garg <sumit.garg@linaro.org> wrote: > > > > > > > > On Wed, 20 Feb 2019 at 16:19, Colin Ian King <colin.king@canonical.com> wrote: > > > > > > > > > > On 20/02/2019 10:37, Ard Biesheuvel wrote: > > > > > > On Wed, 20 Feb 2019 at 11:34, Sumit Garg <sumit.garg@linaro.org> wrote: > > > > > >> > > > > > >> On Wed, 20 Feb 2019 at 14:51, Wei Yongjun <weiyongjun1@huawei.com> 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
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)}, {}
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 <weiyongjun1@huawei.com> --- drivers/char/hw_random/optee-rng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)