lib/raster: Fix Resource Leak issue in cats.c #5398
Conversation
github-actions
bot
added
raster
![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>
before this comment i had already opened PR for CIDs 1208145, 1208146, 1415666, 1415672 in #5407 |
| @@ -151,6 +151,7 @@ static int read_row_ptrs(int nrows, int old, off_t *row_ptr, int fd) | |||
| return 1; | |||
|
|
|||
| badread: | |||
| G_free(buf); | |||
There was a problem hiding this comment.
buf is only malloced in L129, which means the goto statements in L112, L124 and L126 will free NULL. Although that is possible (given if buf is initialised at definition), I'd say it is clearer if we change the use of goto in those lines to return -1.
| if (type < 0) | ||
| if (type < 0) { | ||
| G_free(rname); | ||
| G_free(rmapset); |
There was a problem hiding this comment.
This one is trickier than first appears.
The typical (perhaps exclusive) use of Rast_is_reclass() is by passing (static) arrays for rname and rmapset:
char rname[GNAME_MAX], rmapset[GMAPSET_MAX];
int is_reclass = (Rast_is_reclass(name, mapset, rname, rmapset) > 0);However reclass_type(), unfortunately allows for using both a static and a dynamic array, by either strcpy to the static array or by allocating a new string with G_store. Only in the latter case, we are allowed to free the memory. Given that the latter case, currently never happens, we should skip CID 1208055 and 1208056 this time.
(A possible solution, for another PR, could be to update the documentation of Rast_is_reclass() to emphasise that rname and rmapset must be a pointer to an (allocated) character array; update reclass_type() according to this fact (remove G_store etc.); and make sure all calls to Rast_is_reclass() use static arrays which are NUL initialised.)
This pull request fixes issue identified by Coverity Scan (CID : 1270327)
Used G_free() to fix this issue.