i.ortho.photo: Fix Resource Leak in find_init.c #4388
Conversation
|
|
||
| element = (char *)G_malloc(80 * sizeof(char)); | ||
|
|
||
| if (group == NULL || *group == 0) | ||
| if (group == NULL || *group == 0) { | ||
| G_free(element); |
There was a problem hiding this comment.
you could return 0 before allocating element
| return 0; | ||
| } | ||
| sprintf(element, "group/%s", group); |
There was a problem hiding this comment.
For another PR: use snprintf instead of sprintf.
| sprintf(element, "group/%s", group); | |
| snprintf(element, ELEMENT_BUFFER_SIZE, "group/%s", group); |
with #define ELEMENT_BUFFER_SIZE 80 also used for malloc instead of a magic number
There was a problem hiding this comment.
is it okay to do the changes in the same PR it will save the workflow check time
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| return G_find_file(element, "INIT_EXP", G_mapset()) != NULL; | ||
| } | ||
|
|
||
| element = (char *)G_malloc(ELEMENT_BUFFER_SIZE * sizeof(char)); |
There was a problem hiding this comment.
The GNAME_MAX discussed in other PR applies here too. No need to introduce another constant. Technically, INAME_LEN should be used for a group name, but I think we will remove INAME_LEN in favor of GNAME_MAX, so here use GNAME_MAX.
If anyone thinks otherwise, please let me know.
| if (group == NULL || *group == 0) { | ||
| return 0; | ||
| sprintf(element, "group/%s", group); | ||
| return G_find_file(element, "INIT_EXP", G_mapset()) != NULL; | ||
| } |
There was a problem hiding this comment.
Please remove the {} which was not originally there and there is no need to add it (to have easier to read diff).
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| sprintf(element, "group/%s", group); | ||
| return G_find_file(element, "INIT_EXP", G_mapset()) != NULL; | ||
|
|
||
| element = (char *)G_malloc(GNAME_MAX * sizeof(char)); |
There was a problem hiding this comment.
I think it would be better to declare element as:
char element[GNAME_MAX];then no need to malloc, free...
There was a problem hiding this comment.
As group may be of size GNAME_MAX
grass/imagery/i.ortho.photo/lib/orthophoto.h
Lines 93 to 94 in 2098577
snprintf may truncate, so you should check for that too.
This pull request addresses resource leak issue identified by coverity scan (CID : 1207903).
Replaced the direct return of G_find_file with a variable (file_exists) to store the result before freeing memory.