[PATCH] mmc_cmds.c: fix warning on uninitialized 'cnt'

[PATCH] mmc_cmds.c: fix warning on uninitialized 'cnt'

[Giulio Benetti]

Since -Werror is enabled by default build fails with:
```
In function '__bswap_32',
    inlined from 'do_rpmb_write_block' at mmc_cmds.c:2293:27:
/home/autobuild/autobuild/instance-15/output-1/host/aarch64-buildroot-linux-gnu/sysroot/usr/include/bits/byteswap.h:52:10: error: 'cnt' may be used uninitialized [-Werror=maybe-uninitialized]
   52 |   return __builtin_bswap32 (__bsx);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
mmc_cmds.c: In function 'do_rpmb_write_block':
mmc_cmds.c:2270:22: note: 'cnt' was declared here
 2270 |         unsigned int cnt;
      |                      ^~~
cc1: all warnings being treated as errors
```
So let's fix the warning by initializing cnt local variable.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 mmc_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mmc_cmds.c b/mmc_cmds.c
index 12b7802..777d649 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
 	int ret, dev_fd, key_fd, data_fd;
 	unsigned char key[32];
 	uint16_t addr;
-	unsigned int cnt;
+	unsigned int cnt = 0;
 	struct rpmb_frame frame_in = {
 		.req_resp    = htobe16(MMC_RPMB_WRITE),
 		.block_count = htobe16(1)
-- 
2.34.1

RE: [PATCH] mmc_cmds.c: fix warning on uninitialized 'cnt'

[Avri Altman]

> Since -Werror is enabled by default build fails with:
> ```
> In function '__bswap_32',
>     inlined from 'do_rpmb_write_block' at mmc_cmds.c:2293:27:
> /home/autobuild/autobuild/instance-15/output-1/host/aarch64-buildroot-
> linux-gnu/sysroot/usr/include/bits/byteswap.h:52:10: error: 'cnt' may be
> used uninitialized [-Werror=maybe-uninitialized]
>    52 |   return __builtin_bswap32 (__bsx);
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~
> mmc_cmds.c: In function 'do_rpmb_write_block':
> mmc_cmds.c:2270:22: note: 'cnt' was declared here
>  2270 |         unsigned int cnt;
>       |                      ^~~
> cc1: all warnings being treated as errors ``` So let's fix the warning by
> initializing cnt local variable.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Please prefix you subject with "mmc-utils:"

Thanks,
Avri

> ---
>  mmc_cmds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 12b7802..777d649 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
>         int ret, dev_fd, key_fd, data_fd;
>         unsigned char key[32];
>         uint16_t addr;
> -       unsigned int cnt;
> +       unsigned int cnt = 0;
>         struct rpmb_frame frame_in = {
>                 .req_resp    = htobe16(MMC_RPMB_WRITE),
>                 .block_count = htobe16(1)
> --
> 2.34.1

Re: [PATCH] mmc_cmds.c: fix warning on uninitialized 'cnt'

[Giulio Benetti]

On 18/09/22 14:34, Avri Altman wrote:
>> Since -Werror is enabled by default build fails with:
>> ```
>> In function '__bswap_32',
>>      inlined from 'do_rpmb_write_block' at mmc_cmds.c:2293:27:
>> /home/autobuild/autobuild/instance-15/output-1/host/aarch64-buildroot-
>> linux-gnu/sysroot/usr/include/bits/byteswap.h:52:10: error: 'cnt' may be
>> used uninitialized [-Werror=maybe-uninitialized]
>>     52 |   return __builtin_bswap32 (__bsx);
>>        |          ^~~~~~~~~~~~~~~~~~~~~~~~~
>> mmc_cmds.c: In function 'do_rpmb_write_block':
>> mmc_cmds.c:2270:22: note: 'cnt' was declared here
>>   2270 |         unsigned int cnt;
>>        |                      ^~~
>> cc1: all warnings being treated as errors ``` So let's fix the warning by
>> initializing cnt local variable.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Please prefix you subject with "mmc-utils:"

Ok, thank you. Going to send V2

Best regards
-- 
Giulio Benetti
Benetti Engineering sas

> Thanks,
> Avri
> 
>> ---
>>   mmc_cmds.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mmc_cmds.c b/mmc_cmds.c
>> index 12b7802..777d649 100644
>> --- a/mmc_cmds.c
>> +++ b/mmc_cmds.c
>> @@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
>>          int ret, dev_fd, key_fd, data_fd;
>>          unsigned char key[32];
>>          uint16_t addr;
>> -       unsigned int cnt;
>> +       unsigned int cnt = 0;
>>          struct rpmb_frame frame_in = {
>>                  .req_resp    = htobe16(MMC_RPMB_WRITE),
>>                  .block_count = htobe16(1)
>> --
>> 2.34.1
> 

[PATCH v2] mmc-utils: mmc_cmds.c: fix warning on uninitialized 'cnt'

[Giulio Benetti]

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
V1->V2:
* prefix subjet with 'mmc-utils:' as pointed by Avri Altman
---
 mmc_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mmc_cmds.c b/mmc_cmds.c
index 12b7802..777d649 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
 	int ret, dev_fd, key_fd, data_fd;
 	unsigned char key[32];
 	uint16_t addr;
-	unsigned int cnt;
+	unsigned int cnt = 0;
 	struct rpmb_frame frame_in = {
 		.req_resp    = htobe16(MMC_RPMB_WRITE),
 		.block_count = htobe16(1)
-- 
2.34.1

RE: [PATCH v2] mmc-utils: mmc_cmds.c: fix warning on uninitialized 'cnt'

[Avri Altman]

You forgot to include your commit log.
Also no need to include the module name - just mmc-utils: fix warning on uninitialized 'cnt'

Please feel free to add my reviewed-by once you do.

Thanks,
Avri

> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
> V1->V2:
> * prefix subjet with 'mmc-utils:' as pointed by Avri Altman
> ---
>  mmc_cmds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 12b7802..777d649 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
>         int ret, dev_fd, key_fd, data_fd;
>         unsigned char key[32];
>         uint16_t addr;
> -       unsigned int cnt;
> +       unsigned int cnt = 0;
>         struct rpmb_frame frame_in = {
>                 .req_resp    = htobe16(MMC_RPMB_WRITE),
>                 .block_count = htobe16(1)
> --
> 2.34.1

Re: [PATCH v2] mmc-utils: mmc_cmds.c: fix warning on uninitialized 'cnt'

[Arnd Bergmann]

On Sun, Sep 18, 2022, at 2:42 PM, Giulio Benetti wrote:
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> ---
> V1->V2:
> * prefix subjet with 'mmc-utils:' as pointed by Avri Altman
> ---
>  mmc_cmds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 12b7802..777d649 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
>  	int ret, dev_fd, key_fd, data_fd;
>  	unsigned char key[32];
>  	uint16_t addr;
> -	unsigned int cnt;
> +	unsigned int cnt = 0;
>  	struct rpmb_frame frame_in = {
>  		.req_resp    = htobe16(MMC_RPMB_WRITE),
>  		.block_count = htobe16(1)

I think this is the wrong place to initialize it, I would
suggest doing it inside of rpmb_read_counter() when returning
an error, to make it clear that this is not the intended
counter but something that is not actually meant to be used.

If you initialize local variables to bogus values, you remove
the possibility for the compiler to warn a function actually uses
something without initialization.

       Arnd

Re: [PATCH v2] mmc-utils: mmc_cmds.c: fix warning on uninitialized 'cnt'

[Giulio Benetti]

Hi Arnd,

On 18/09/22 16:13, Arnd Bergmann wrote:
> On Sun, Sep 18, 2022, at 2:42 PM, Giulio Benetti wrote:
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
>> ---
>> V1->V2:
>> * prefix subjet with 'mmc-utils:' as pointed by Avri Altman
>> ---
>>   mmc_cmds.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mmc_cmds.c b/mmc_cmds.c
>> index 12b7802..777d649 100644
>> --- a/mmc_cmds.c
>> +++ b/mmc_cmds.c
>> @@ -2436,7 +2436,7 @@ int do_rpmb_write_block(int nargs, char **argv)
>>   	int ret, dev_fd, key_fd, data_fd;
>>   	unsigned char key[32];
>>   	uint16_t addr;
>> -	unsigned int cnt;
>> +	unsigned int cnt = 0;
>>   	struct rpmb_frame frame_in = {
>>   		.req_resp    = htobe16(MMC_RPMB_WRITE),
>>   		.block_count = htobe16(1)
> 
> I think this is the wrong place to initialize it, I would
> suggest doing it inside of rpmb_read_counter() when returning
> an error, to make it clear that this is not the intended
> counter but something that is not actually meant to be used.
> 
> If you initialize local variables to bogus values, you remove
> the possibility for the compiler to warn a function actually uses
> something without initialization.

Totally right, thank you.

-- 
Giulio Benetti
Benetti Engineering sas

[PATCH v3] mmc-utils: fix warning on uninitialized 'cnt'

[Giulio Benetti]

When building following warning shows up:
```
In function '__bswap_32',
    inlined from 'do_rpmb_write_block' at mmc_cmds.c:2293:27:
/home/autobuild/autobuild/instance-15/output-1/host/aarch64-buildroot-linux-gnu/sysroot/usr/include/bits/byteswap.h:52:10: error: 'cnt' may be used uninitialized [-Werror=maybe-uninitialized]
   52 |   return __builtin_bswap32 (__bsx);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
mmc_cmds.c: In function 'do_rpmb_write_block':
mmc_cmds.c:2270:22: note: 'cnt' was declared here
2270 |         unsigned int cnt;
      |                      ^~~
cc1: all warnings being treated as errors
```
This is due to function rpmb_read_counter() that doesn't set its
argument 'unsigned int *cnt' in all return points. So let's set
*cnt to 0 in the return point that misses to initialize it.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
V1->V2:
* prefix subject with 'mmc-utils:' as pointed by Avri Altman
V2->V3:
* add missing commit as pointed by Avri Altman
* initialize pointer inside rpmb_read_counter() as suggested by Arnd Bergmann
---
 mmc_cmds.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mmc_cmds.c b/mmc_cmds.c
index 12b7802..4d203ef 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2238,8 +2238,10 @@ int rpmb_read_counter(int dev_fd, unsigned int *cnt)
 	}
 
 	/* Check RPMB response */
-	if (frame_out.result != 0)
+	if (frame_out.result != 0) {
+		*cnt = 0;
 		return be16toh(frame_out.result);
+	}
 
 	*cnt = be32toh(frame_out.write_counter);
 
-- 
2.34.1

RE: [PATCH v3] mmc-utils: fix warning on uninitialized 'cnt'

[Avri Altman]

> When building following warning shows up:
> ```
> In function '__bswap_32',
>     inlined from 'do_rpmb_write_block' at mmc_cmds.c:2293:27:
> /home/autobuild/autobuild/instance-15/output-1/host/aarch64-buildroot-
> linux-gnu/sysroot/usr/include/bits/byteswap.h:52:10: error: 'cnt' may be
> used uninitialized [-Werror=maybe-uninitialized]
>    52 |   return __builtin_bswap32 (__bsx);
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~
> mmc_cmds.c: In function 'do_rpmb_write_block':
> mmc_cmds.c:2270:22: note: 'cnt' was declared here
> 2270 |         unsigned int cnt;
>       |                      ^~~
> cc1: all warnings being treated as errors ``` This is due to function
> rpmb_read_counter() that doesn't set its argument 'unsigned int *cnt' in all
> return points. So let's set *cnt to 0 in the return point that misses to initialize
> it.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>


> ---
> V1->V2:
> * prefix subject with 'mmc-utils:' as pointed by Avri Altman
> V2->V3:
> * add missing commit as pointed by Avri Altman
> * initialize pointer inside rpmb_read_counter() as suggested by Arnd
> Bergmann
> ---
>  mmc_cmds.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 12b7802..4d203ef 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2238,8 +2238,10 @@ int rpmb_read_counter(int dev_fd, unsigned int
> *cnt)
>         }
> 
>         /* Check RPMB response */
> -       if (frame_out.result != 0)
> +       if (frame_out.result != 0) {
> +               *cnt = 0;
>                 return be16toh(frame_out.result);
> +       }
> 
>         *cnt = be32toh(frame_out.write_counter);
> 
> --
> 2.34.1

Re: [PATCH v3] mmc-utils: fix warning on uninitialized 'cnt'

[Ulf Hansson]

On Sun, 18 Sept 2022 at 18:17, Giulio Benetti
<giulio.benetti@benettiengineering.com> wrote:
>
> When building following warning shows up:
> ```
> In function '__bswap_32',
>     inlined from 'do_rpmb_write_block' at mmc_cmds.c:2293:27:
> /home/autobuild/autobuild/instance-15/output-1/host/aarch64-buildroot-linux-gnu/sysroot/usr/include/bits/byteswap.h:52:10: error: 'cnt' may be used uninitialized [-Werror=maybe-uninitialized]
>    52 |   return __builtin_bswap32 (__bsx);
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~
> mmc_cmds.c: In function 'do_rpmb_write_block':
> mmc_cmds.c:2270:22: note: 'cnt' was declared here
> 2270 |         unsigned int cnt;
>       |                      ^~~
> cc1: all warnings being treated as errors
> ```
> This is due to function rpmb_read_counter() that doesn't set its
> argument 'unsigned int *cnt' in all return points. So let's set
> *cnt to 0 in the return point that misses to initialize it.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

Applied to git.kernel.org/pub/scm//utils/mmc/mmc-utils.git master, thanks!

Kind regards
Uffe


> ---
> V1->V2:
> * prefix subject with 'mmc-utils:' as pointed by Avri Altman
> V2->V3:
> * add missing commit as pointed by Avri Altman
> * initialize pointer inside rpmb_read_counter() as suggested by Arnd Bergmann
> ---
>  mmc_cmds.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 12b7802..4d203ef 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -2238,8 +2238,10 @@ int rpmb_read_counter(int dev_fd, unsigned int *cnt)
>         }
>
>         /* Check RPMB response */
> -       if (frame_out.result != 0)
> +       if (frame_out.result != 0) {
> +               *cnt = 0;
>                 return be16toh(frame_out.result);
> +       }
>
>         *cnt = be32toh(frame_out.write_counter);
>
> --
> 2.34.1
>