[U-Boot] [PATCH] libfdt: Initialize the stack variable

[J. William Campbell]

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 at 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 at 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;