* [U-Boot] [PATCH] libfdt: Initialize the stack variable
@ 2017-08-30 5:15 tien.fong.chee at intel.com
2017-08-30 13:31 ` J. William Campbell
0 siblings, 1 reply; 9+ messages in thread
From: tien.fong.chee at intel.com @ 2017-08-30 5:15 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Report Coverity log:
The code uses a variable that has not
been initialized, leading to unpredictable
or unintended results.
Reported-by: Coverity (CID: 60519)
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
lib/libfdt/fdt_wip.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
index 45fb964..01adad0 100644
--- a/lib/libfdt/fdt_wip.c
+++ b/lib/libfdt/fdt_wip.c
@@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
struct fdt_region region[], int max_regions,
char *path, int path_len, int add_string_tab)
{
- int stack[FDT_MAX_DEPTH];
+ int stack[FDT_MAX_DEPTH] = { 0 };
char *end;
int nextoffset = 0;
uint32_t tag;
--
1.7.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-08-30 5:15 [U-Boot] [PATCH] libfdt: Initialize the stack variable tien.fong.chee at intel.com
@ 2017-08-30 13:31 ` J. William Campbell
2017-09-05 3:41 ` Chee, Tien Fong
0 siblings, 1 reply; 9+ messages in thread
From: J. William Campbell @ 2017-08-30 13:31 UTC (permalink / raw)
To: u-boot
On 8/29/2017 10:15 PM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Report Coverity log:
> The code uses a variable that has not
> been initialized, leading to unpredictable
> or unintended results.
>
> Reported-by: Coverity (CID: 60519)
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> lib/libfdt/fdt_wip.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
> index 45fb964..01adad0 100644
> --- a/lib/libfdt/fdt_wip.c
> +++ b/lib/libfdt/fdt_wip.c
> @@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
> struct fdt_region region[], int max_regions,
> char *path, int path_len, int add_string_tab)
> {
> - int stack[FDT_MAX_DEPTH];
> + int stack[FDT_MAX_DEPTH] = { 0 };
It seems to me that one of three things must be true. 1) Coverity can't
correctly analyze the code and stack[] is not used in an un-initialized
manner, 2) stack is used in an un-initialized manner but the result is
not used in that case and is a "don't care" or 3) there is a bug in the
code. It seems that just initializing the variable to 0 is a "Bad
Idea(tm)". If it is case 1 or 2, there should be a Coverity code
annotation comment added to that effect, and if it is case 3, it should
be fixed. Initializing this variable makes the binary larger to no
purpose unless there is a bug already.
Best Regards,
J. William Campbell
> char *end;
> int nextoffset = 0;
> uint32_t tag;
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-08-30 13:31 ` J. William Campbell
@ 2017-09-05 3:41 ` Chee, Tien Fong
2017-09-05 3:58 ` J. William Campbell
0 siblings, 1 reply; 9+ messages in thread
From: Chee, Tien Fong @ 2017-09-05 3:41 UTC (permalink / raw)
To: u-boot
On Rab, 2017-08-30 at 06:31 -0700, J. William Campbell wrote:
> On 8/29/2017 10:15 PM, tien.fong.chee at intel.com wrote:
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Report Coverity log:
> > The code uses a variable that has not
> > been initialized, leading to unpredictable
> > or unintended results.
> >
> > Reported-by: Coverity (CID: 60519)
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > lib/libfdt/fdt_wip.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
> > index 45fb964..01adad0 100644
> > --- a/lib/libfdt/fdt_wip.c
> > +++ b/lib/libfdt/fdt_wip.c
> > @@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char *
> > const inc[], int inc_count,
> > struct fdt_region region[], int max_regions,
> > char *path, int path_len, int
> > add_string_tab)
> > {
> > - int stack[FDT_MAX_DEPTH];
> > + int stack[FDT_MAX_DEPTH] = { 0 };
> It seems to me that one of three things must be true. 1) Coverity
> can't
> correctly analyze the code and stack[] is not used in an un-
> initialized
> manner, 2) stack is used in an un-initialized manner but the result
> is
> not used in that case and is a "don't care" or 3) there is a bug in
> the
> code. It seems that just initializing the variable to 0 is a "Bad
> Idea(tm)". If it is case 1 or 2, there should be a Coverity code
> annotation comment added to that effect, and if it is case 3, it
> should
> be fixed. Initializing this variable makes the binary larger to no
> purpose unless there is a bug already.
>
> Best Regards,
> J. William Campbell
Yeah, i agree with you, state machine design should ensure stack[] is
not used in a uninitialized manner. Hence, i need input from whom
familiar with this function, whether this warning fall in anyone of
these conditions. If we just direct init the stack[], and this solution
will make extra 128 bytes in binary, but having variable with default
value is also good pratice from software quality perspective.
> >
> > char *end;
> > int nextoffset = 0;
> > uint32_t tag;
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-09-05 3:41 ` Chee, Tien Fong
@ 2017-09-05 3:58 ` J. William Campbell
2017-09-15 19:25 ` sjg at google.com
0 siblings, 1 reply; 9+ messages in thread
From: J. William Campbell @ 2017-09-05 3:58 UTC (permalink / raw)
To: u-boot
On 9/4/2017 8:41 PM, Chee, Tien Fong wrote:
> On Rab, 2017-08-30 at 06:31 -0700, J. William Campbell wrote:
>> On 8/29/2017 10:15 PM, tien.fong.chee at intel.com wrote:
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> Report Coverity log:
>>> The code uses a variable that has not
>>> been initialized, leading to unpredictable
>>> or unintended results.
>>>
>>> Reported-by: Coverity (CID: 60519)
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>> ---
>>> lib/libfdt/fdt_wip.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
>>> index 45fb964..01adad0 100644
>>> --- a/lib/libfdt/fdt_wip.c
>>> +++ b/lib/libfdt/fdt_wip.c
>>> @@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char *
>>> const inc[], int inc_count,
>>> struct fdt_region region[], int max_regions,
>>> char *path, int path_len, int
>>> add_string_tab)
>>> {
>>> - int stack[FDT_MAX_DEPTH];
>>> + int stack[FDT_MAX_DEPTH] = { 0 };
>> It seems to me that one of three things must be true. 1) Coverity
>> can't
>> correctly analyze the code and stack[] is not used in an un-
>> initialized
>> manner, 2) stack is used in an un-initialized manner but the result
>> is
>> not used in that case and is a "don't care" or 3) there is a bug in
>> the
>> code. It seems that just initializing the variable to 0 is a "Bad
>> Idea(tm)". If it is case 1 or 2, there should be a Coverity code
>> annotation comment added to that effect, and if it is case 3, it
>> should
>> be fixed. Initializing this variable makes the binary larger to no
>> purpose unless there is a bug already.
>>
>> Best Regards,
>> J. William Campbell
> Yeah, i agree with you, state machine design should ensure stack[] is
> not used in a uninitialized manner. Hence, i need input from whom
> familiar with this function, whether this warning fall in anyone of
> these conditions. If we just direct init the stack[], and this solution
> will make extra 128 bytes in binary, but having variable with default
> value is also good pratice from software quality perspective.
Yes, if the default value has a rationale. On the surface, there is no
way to know that 0 is a "good" initial value. There may be a reason that
it is, but if we don't know for sure, it is just a "random" number. I
hope whoever wrote this will speak up and say why the variable is never
used before it is initialized. Thank you for being so diligent.
Best Regards,
Bill Campbell
>>> char *end;
>>> int nextoffset = 0;
>>> uint32_t tag;
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-09-05 3:58 ` J. William Campbell
@ 2017-09-15 19:25 ` sjg at google.com
0 siblings, 0 replies; 9+ messages in thread
From: sjg at google.com @ 2017-09-15 19:25 UTC (permalink / raw)
To: u-boot
On 9/4/2017 8:41 PM, Chee, Tien Fong wrote:
> On Rab, 2017-08-30 at 06:31 -0700, J. William Campbell wrote:
>> On 8/29/2017 10:15 PM, tien.fong.chee at intel.com wrote:
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> Report Coverity log:
>>> The code uses a variable that has not
>>> been initialized, leading to unpredictable
>>> or unintended results.
>>>
>>> Reported-by: Coverity (CID: 60519)
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>> ---
>>> lib/libfdt/fdt_wip.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
Applied to u-boot-fdt thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] libfdt: Initialize the stack variable
@ 2017-08-24 5:53 tien.fong.chee at intel.com
2017-08-25 1:04 ` Tom Rini
0 siblings, 1 reply; 9+ messages in thread
From: tien.fong.chee at intel.com @ 2017-08-24 5:53 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Report Coverity log:
The code uses a variable that has not
been initialized, leading to unpredictable
or unintended results.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
lib/libfdt/fdt_wip.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
index 45fb964..01adad0 100644
--- a/lib/libfdt/fdt_wip.c
+++ b/lib/libfdt/fdt_wip.c
@@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
struct fdt_region region[], int max_regions,
char *path, int path_len, int add_string_tab)
{
- int stack[FDT_MAX_DEPTH];
+ int stack[FDT_MAX_DEPTH] = { 0 };
char *end;
int nextoffset = 0;
uint32_t tag;
--
1.7.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-08-24 5:53 tien.fong.chee at intel.com
@ 2017-08-25 1:04 ` Tom Rini
2017-08-25 4:01 ` Chee, Tien Fong
2017-08-30 5:01 ` Chee, Tien Fong
0 siblings, 2 replies; 9+ messages in thread
From: Tom Rini @ 2017-08-25 1:04 UTC (permalink / raw)
To: u-boot
On Thu, Aug 24, 2017 at 01:53:57PM +0800, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Report Coverity log:
> The code uses a variable that has not
> been initialized, leading to unpredictable
> or unintended results.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> lib/libfdt/fdt_wip.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
> index 45fb964..01adad0 100644
> --- a/lib/libfdt/fdt_wip.c
> +++ b/lib/libfdt/fdt_wip.c
> @@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char * const inc[], int inc_count,
> struct fdt_region region[], int max_regions,
> char *path, int path_len, int add_string_tab)
> {
> - int stack[FDT_MAX_DEPTH];
> + int stack[FDT_MAX_DEPTH] = { 0 };
> char *end;
> int nextoffset = 0;
> uint32_t tag;
Since this comes from libfdt, have you checked there as well? And in
general, we use a Reported-by: Coverity (CID: xxxx) for issues. BTW, if
you would like access to the community version of Coverity, please sign
up at https://scan.coverity.com/projects/das-u-boot?tab=overview thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170824/f831b64e/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-08-25 1:04 ` Tom Rini
@ 2017-08-25 4:01 ` Chee, Tien Fong
2017-08-30 5:01 ` Chee, Tien Fong
1 sibling, 0 replies; 9+ messages in thread
From: Chee, Tien Fong @ 2017-08-25 4:01 UTC (permalink / raw)
To: u-boot
On Kha, 2017-08-24 at 21:04 -0400, Tom Rini wrote:
> On Thu, Aug 24, 2017 at 01:53:57PM +0800, tien.fong.chee at intel.com
> wrote:
>
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Report Coverity log:
> > The code uses a variable that has not
> > been initialized, leading to unpredictable
> > or unintended results.
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > lib/libfdt/fdt_wip.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
> > index 45fb964..01adad0 100644
> > --- a/lib/libfdt/fdt_wip.c
> > +++ b/lib/libfdt/fdt_wip.c
> > @@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char *
> > const inc[], int inc_count,
> > struct fdt_region region[], int max_regions,
> > char *path, int path_len, int add_string_tab)
> > {
> > - int stack[FDT_MAX_DEPTH];
> > + int stack[FDT_MAX_DEPTH] = { 0 };
> > char *end;
> > int nextoffset = 0;
> > uint32_t tag;
> Since this comes from libfdt, have you checked there as well? And in
> general, we use a Reported-by: Coverity (CID: xxxx) for issues. BTW,
> if
> you would like access to the community version of Coverity, please
> sign
> up at https://scan.coverity.com/projects/das-u-boot?tab=overview
> thanks!
I think we didn't check all of them, our coverity only check against
our socfpga use cases. But, i failed to "Add me to project", i can't
view all the defects. Does community coverity found the similar warning
as this patch?
Thanks.
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] libfdt: Initialize the stack variable
2017-08-25 1:04 ` Tom Rini
2017-08-25 4:01 ` Chee, Tien Fong
@ 2017-08-30 5:01 ` Chee, Tien Fong
1 sibling, 0 replies; 9+ messages in thread
From: Chee, Tien Fong @ 2017-08-30 5:01 UTC (permalink / raw)
To: u-boot
On Kha, 2017-08-24 at 21:04 -0400, Tom Rini wrote:
> On Thu, Aug 24, 2017 at 01:53:57PM +0800, tien.fong.chee at intel.com
> wrote:
>
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Report Coverity log:
> > The code uses a variable that has not
> > been initialized, leading to unpredictable
> > or unintended results.
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > lib/libfdt/fdt_wip.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/libfdt/fdt_wip.c b/lib/libfdt/fdt_wip.c
> > index 45fb964..01adad0 100644
> > --- a/lib/libfdt/fdt_wip.c
> > +++ b/lib/libfdt/fdt_wip.c
> > @@ -115,7 +115,7 @@ int fdt_find_regions(const void *fdt, char *
> > const inc[], int inc_count,
> > struct fdt_region region[], int max_regions,
> > char *path, int path_len, int add_string_tab)
> > {
> > - int stack[FDT_MAX_DEPTH];
> > + int stack[FDT_MAX_DEPTH] = { 0 };
> > char *end;
> > int nextoffset = 0;
> > uint32_t tag;
> Since this comes from libfdt, have you checked there as well? And in
> general, we use a Reported-by: Coverity (CID: xxxx) for issues. BTW,
> if
> you would like access to the community version of Coverity, please
> sign
> up at https://scan.coverity.com/projects/das-u-boot?tab=overview
> thanks!
>
Okay, i can add myself to Das U-boot coverity project finally. This
warning is reported by CID: 60519. I will send out another patch with
Reported-by in commit messages.
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-15 19:25 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-30 5:15 [U-Boot] [PATCH] libfdt: Initialize the stack variable tien.fong.chee at intel.com
2017-08-30 13:31 ` J. William Campbell
2017-09-05 3:41 ` Chee, Tien Fong
2017-09-05 3:58 ` J. William Campbell
2017-09-15 19:25 ` sjg at google.com
-- strict thread matches above, loose matches on Subject: below --
2017-08-24 5:53 tien.fong.chee at intel.com
2017-08-25 1:04 ` Tom Rini
2017-08-25 4:01 ` Chee, Tien Fong
2017-08-30 5:01 ` Chee, Tien Fong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.