diff mbox series

[1/1] cmd: upl: initialize unit test state

Message ID 20241102143020.129450-1-heinrich.schuchardt@canonical.com
State New
Delegated to: Tom Rini
Headers show
Series [1/1] cmd: upl: initialize unit test state | expand

Commit Message

Heinrich Schuchardt Nov. 2, 2024, 2:30 p.m. UTC
do_upl_write() calls upl_get_test_data() which may increment the fail
count in the unit test state. We should initialize it.

Addresses-Coverity-ID: 510465 Uninitialized scalar variable
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 cmd/upl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Neha Malcom Francis Nov. 4, 2024, 5:01 a.m. UTC | #1
Hi Heinrich

On 02/11/24 20:00, Heinrich Schuchardt wrote:
> do_upl_write() calls upl_get_test_data() which may increment the fail
> count in the unit test state. We should initialize it.
> 
> Addresses-Coverity-ID: 510465 Uninitialized scalar variable
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>   cmd/upl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmd/upl.c b/cmd/upl.c
> index 4996f36c787..c9a823bbc06 100644
> --- a/cmd/upl.c
> +++ b/cmd/upl.c
> @@ -50,7 +50,7 @@ static int do_upl_write(struct cmd_tbl *cmdtp, int flag, int argc,
>   			char *const argv[])
>   {
>   	struct upl s_upl, *upl = &s_upl;
> -	struct unit_test_state uts;
> +	struct unit_test_state uts = { 0 };
>   	struct abuf buf;
>   	oftree tree;
>   	ulong addr;

Reviewed-by: Neha Malcom Francis <n-francis@ti.com>
Simon Glass Nov. 5, 2024, 3:12 p.m. UTC | #2
Hi Heinrich,

On Sat, 2 Nov 2024 at 08:30, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> do_upl_write() calls upl_get_test_data() which may increment the fail
> count in the unit test state. We should initialize it.
>
> Addresses-Coverity-ID: 510465 Uninitialized scalar variable
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  cmd/upl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmd/upl.c b/cmd/upl.c
> index 4996f36c787..c9a823bbc06 100644
> --- a/cmd/upl.c
> +++ b/cmd/upl.c
> @@ -50,7 +50,7 @@ static int do_upl_write(struct cmd_tbl *cmdtp, int flag, int argc,
>                         char *const argv[])
>  {
>         struct upl s_upl, *upl = &s_upl;
> -       struct unit_test_state uts;
> +       struct unit_test_state uts = { 0 };
>         struct abuf buf;
>         oftree tree;
>         ulong addr;
> --
> 2.45.2
>

This already exists. A memset() is in upl_init() which is called from
this function.

Regards,
Simon
Heinrich Schuchardt Nov. 6, 2024, 4 p.m. UTC | #3
On 05.11.24 16:12, Simon Glass wrote:
> Hi Heinrich,
> 
> On Sat, 2 Nov 2024 at 08:30, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
>>
>> do_upl_write() calls upl_get_test_data() which may increment the fail
>> count in the unit test state. We should initialize it.
>>
>> Addresses-Coverity-ID: 510465 Uninitialized scalar variable
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>   cmd/upl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/cmd/upl.c b/cmd/upl.c
>> index 4996f36c787..c9a823bbc06 100644
>> --- a/cmd/upl.c
>> +++ b/cmd/upl.c
>> @@ -50,7 +50,7 @@ static int do_upl_write(struct cmd_tbl *cmdtp, int flag, int argc,
>>                          char *const argv[])
>>   {
>>          struct upl s_upl, *upl = &s_upl;
>> -       struct unit_test_state uts;
>> +       struct unit_test_state uts = { 0 };
>>          struct abuf buf;
>>          oftree tree;
>>          ulong addr;
>> --
>> 2.45.2
>>
> 
> This already exists. A memset() is in upl_init() which is called from
> this function.

My patch is about initializing the unit_test_state variable uts (and not 
upl).

upl_init() is called for upl and not for the unit_test_state.

Best regards

Heinrich
Simon Glass Nov. 7, 2024, 1:44 p.m. UTC | #4
Hi Heinrich,

On Wed, 6 Nov 2024 at 09:00, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 05.11.24 16:12, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sat, 2 Nov 2024 at 08:30, Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> wrote:
> >>
> >> do_upl_write() calls upl_get_test_data() which may increment the fail
> >> count in the unit test state. We should initialize it.
> >>
> >> Addresses-Coverity-ID: 510465 Uninitialized scalar variable
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> >> ---
> >>   cmd/upl.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/cmd/upl.c b/cmd/upl.c
> >> index 4996f36c787..c9a823bbc06 100644
> >> --- a/cmd/upl.c
> >> +++ b/cmd/upl.c
> >> @@ -50,7 +50,7 @@ static int do_upl_write(struct cmd_tbl *cmdtp, int flag, int argc,
> >>                          char *const argv[])
> >>   {
> >>          struct upl s_upl, *upl = &s_upl;
> >> -       struct unit_test_state uts;
> >> +       struct unit_test_state uts = { 0 };
> >>          struct abuf buf;
> >>          oftree tree;
> >>          ulong addr;
> >> --
> >> 2.45.2
> >>
> >
> > This already exists. A memset() is in upl_init() which is called from
> > this function.
>
> My patch is about initializing the unit_test_state variable uts (and not
> upl).
>
> upl_init() is called for upl and not for the unit_test_state.

Oh, duh, got it. At some point I will have to make time to come back
and tidy up this UPL thing, perhaps when I test it with Tianocore...

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon
diff mbox series

Patch

diff --git a/cmd/upl.c b/cmd/upl.c
index 4996f36c787..c9a823bbc06 100644
--- a/cmd/upl.c
+++ b/cmd/upl.c
@@ -50,7 +50,7 @@  static int do_upl_write(struct cmd_tbl *cmdtp, int flag, int argc,
 			char *const argv[])
 {
 	struct upl s_upl, *upl = &s_upl;
-	struct unit_test_state uts;
+	struct unit_test_state uts = { 0 };
 	struct abuf buf;
 	oftree tree;
 	ulong addr;