Dear Mattias Hansson,
In message 20211123080633.8318-1-hansson.mattias@gmail.com you wrote:
If `sb_load_cmdfile()` fails to open the configuration file it will jump to error handling where the code will try to `fclose()` the FILE pointer which is NULL causing `mkimage` to segfault.
This patch removes the `fclose()` since `fopen()` always returns NULL instead of the file descriptor when failing.
tools/mxsimage.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/tools/mxsimage.c b/tools/mxsimage.c index 002f4b525a..c7bd86ce52 100644 --- a/tools/mxsimage.c +++ b/tools/mxsimage.c @@ -1618,7 +1618,6 @@ static int sb_load_cmdfile(struct sb_image_ctx *ictx) return 0;
err_file:
- fclose(fp); fprintf(stderr, "ERR: Failed to load file "%s"\n", ictx->cfg_filename); return -EINVAL;
The whole code with the goto is ugly. Such an error exit may make sense when used in several places, but here it just makes reading the code more difficult. I suggest to fix this like this:
diff --git a/tools/mxsimage.c b/tools/mxsimage.c index 002f4b525a..801aaa62ce 100644 --- a/tools/mxsimage.c +++ b/tools/mxsimage.c @@ -1595,8 +1595,11 @@ static int sb_load_cmdfile(struct sb_image_ctx *ictx) size_t len;
fp = fopen(ictx->cfg_filename, "r"); - if (!fp) - goto err_file; + if (!fp) { + fprintf(stderr, "ERR: Failed to load file "%s"\n", + ictx->cfg_filename); + return -EINVAL; + }
while ((rlen = getline(&line, &len, fp)) > 0) { memset(&cmd, 0, sizeof(cmd)); @@ -1616,12 +1619,6 @@ static int sb_load_cmdfile(struct sb_image_ctx *ictx) fclose(fp);
return 0; - -err_file: - fclose(fp); - fprintf(stderr, "ERR: Failed to load file "%s"\n", - ictx->cfg_filename); - return -EINVAL; }
static int sb_build_tree_from_cfg(struct sb_image_ctx *ictx)
Thanks.
Best regards,
Wolfgang Denk