libfdt: Initialize the stack variable
PATCH 1/1 libfdt: Initialize the stack variable
From: Tien Fong Chee <[email protected]>
Report Coverity log:
The code uses a variable that has not
been initialized, leading to unpredictable
or unintended results.
--- lib/libfdt/fdt_wip.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
lib/libfdt/fdt_wip.c +1 -1
@@ -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;
Signature · 1 line
1.7.7.4
PATCH 1/1 libfdt: Initialize the stack variable
Quoted 18 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>--- lib/libfdt/fdt_wip.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)lib/libfdt/fdt_wip.c +0
-0
lib/libfdt/fdt_wip.c int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, · line 118
Quoted 5 lines
lib/libfdt/fdt_wip.c +1
-1@@ -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
Quoted 3 lines
lib/libfdt/fdt_wip.c +0
-0char *end;int nextoffset =0 ;uint32_t tag;
Chee
PATCH 1/1 libfdt: Initialize the stack variable
Quoted 42 lines · J. William Campbell
Quoted 24 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>---
lib/libfdt/fdt_wip.c| 2 +-1 files changed, 1 insertions(+), 1 deletions(-)lib/libfdt/fdt_wip.c +0
-0@@ -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, intadd_string_tab)
{lib/libfdt/fdt_wip.c int fdt_find_regions(const void *fdt, char * const inc[], int inc_count, · line 118
Quoted 2 lines
lib/libfdt/fdt_wip.c +1
-1-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.
Quoted 2 lines
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.
Quoted 5 lines
Quoted 4 lines
char *end;
int nextoffset = 0;
uint32_ttag;
PATCH 1/1 libfdt: Initialize the stack variable
Quoted 42 lines · Tien Fong
Quoted 41 lines · J. William Campbell
Quoted 18 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
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 <[email protected]>--- lib/libfdt/fdt_wip.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)lib/libfdt/fdt_wip.c +0
-0lib/libfdt/fdt_wip.c int fdt_find_regions(const void *fdt, char * · line 120
Quoted 7 lines
lib/libfdt/fdt_wip.c +1
-1@@ -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.
Quoted 8 lines
Quoted 2 lines
Best Regards,
J. William CampbellYeah, 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
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
lib/libfdt/fdt_wip.c +0
-0char *end;int nextoffset = 0;
uint32_ttag;
PATCH 1/1 libfdt: Initialize the stack variable
Quoted 9 lines · Tien Fong
Quoted 8 lines · J. William Campbell
Quoted 7 lines · tien.fong.chee at intel.com
From: Tien Fong Chee <[email protected]>
Report Coverity log:
The code uses a variable that has not
been initialized, leading to unpredictable
or unintended results.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Reported-by Coverity (CID: 60519)Signed-off-by Tien Fong Chee <[email protected]>--- lib/libfdt/fdt_wip.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Applied to u-boot-fdt thanks!