| Message ID | 20180609182235.33532-6-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Fix some coverity warnings | expand |
On 06/09/2018 08:22 PM, Simon Glass wrote: > The current code might succeed on the first allocation and fail on the > second. Separate the checks to avoid this problem. > > Of course, free() will never fail and the chances that (when allocating > two small areas) one will succeed and one will fail are just as remote. > But this keeps coverity happy. > > Reported-by: Coverity (CID: 131226) > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > tools/fdtgrep.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c > index c4563e2289..5593b42203 100644 > --- a/tools/fdtgrep.c > +++ b/tools/fdtgrep.c > @@ -133,11 +133,11 @@ static int value_add(struct display_info *disp, struct value_node **headp, > } > > str = strdup(str); > + if (!str) > + goto err_mem; > node = malloc(sizeof(*node)); > - if (!str || !node) { > - fprintf(stderr, "Out of memory\n"); > - return -1; > - } > + if (!node) > + goto err_mem; > node->next = *headp; > node->type = type; > node->include = include; > @@ -145,6 +145,9 @@ static int value_add(struct display_info *disp, struct value_node **headp, > *headp = node; > free(str) is missing here. > return 0; > +err_mem: free(str) is missing here. Best regards Heinrich > + fprintf(stderr, "Out of memory\n"); > + return -1; > } > > static bool util_is_printable_string(const void *data, int len) >
Hi Heinrich, On 9 June 2018 at 13:46, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 06/09/2018 08:22 PM, Simon Glass wrote: >> The current code might succeed on the first allocation and fail on the >> second. Separate the checks to avoid this problem. >> >> Of course, free() will never fail and the chances that (when allocating >> two small areas) one will succeed and one will fail are just as remote. >> But this keeps coverity happy. >> >> Reported-by: Coverity (CID: 131226) >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> tools/fdtgrep.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c >> index c4563e2289..5593b42203 100644 >> --- a/tools/fdtgrep.c >> +++ b/tools/fdtgrep.c >> @@ -133,11 +133,11 @@ static int value_add(struct display_info *disp, struct value_node **headp, >> } >> >> str = strdup(str); >> + if (!str) >> + goto err_mem; >> node = malloc(sizeof(*node)); >> - if (!str || !node) { >> - fprintf(stderr, "Out of memory\n"); >> - return -1; >> - } >> + if (!node) >> + goto err_mem; >> node->next = *headp; >> node->type = type; >> node->include = include; >> @@ -145,6 +145,9 @@ static int value_add(struct display_info *disp, struct value_node **headp, >> *headp = node; >> > > free(str) is missing here. If we free the string then the node will be invalid. That is the point of the strdup(), so that the node can have a copy of the string. > > >> return 0; >> +err_mem: > > free(str) is missing here. But the strdup() failed, so str is NULL and there is nothing to free. > > Best regards > > Heinrich > >> + fprintf(stderr, "Out of memory\n"); >> + return -1; >> } >> >> static bool util_is_printable_string(const void *data, int len) >> > Regards, Simon
diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c index c4563e2289..5593b42203 100644 --- a/tools/fdtgrep.c +++ b/tools/fdtgrep.c @@ -133,11 +133,11 @@ static int value_add(struct display_info *disp, struct value_node **headp, } str = strdup(str); + if (!str) + goto err_mem; node = malloc(sizeof(*node)); - if (!str || !node) { - fprintf(stderr, "Out of memory\n"); - return -1; - } + if (!node) + goto err_mem; node->next = *headp; node->type = type; node->include = include; @@ -145,6 +145,9 @@ static int value_add(struct display_info *disp, struct value_node **headp, *headp = node; return 0; +err_mem: + fprintf(stderr, "Out of memory\n"); + return -1; } static bool util_is_printable_string(const void *data, int len)
The current code might succeed on the first allocation and fail on the second. Separate the checks to avoid this problem. Of course, free() will never fail and the chances that (when allocating two small areas) one will succeed and one will fail are just as remote. But this keeps coverity happy. Reported-by: Coverity (CID: 131226) Signed-off-by: Simon Glass <sjg@chromium.org> --- tools/fdtgrep.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)