[PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
Ralph Siemsen
ralph.siemsen at linaro.org
Mon May 8 19:50:01 CEST 2023
On Sun, May 07, 2023 at 06:08:33PM +0200, Marek Vasut wrote:
>[...]
>
>>+static int spkgimage_parse_config_file(char *filename)
>>+{
>>+ FILE *fcfg;
>>+ char line[256];
>>+ size_t line_num = 0;
>>+
>>+ fcfg = fopen(filename, "r");
>>+ if (!fcfg)
>>+ return -EINVAL;
>>+
>>+ conf = calloc(1, sizeof(struct config_file));
>>+ if (!conf)
>>+ return -ENOMEM;
>>+
>>+ while (fgets(line, sizeof(line), fcfg)) {
>>+ line_num += 1;
>>+
>>+ /* Skip blank lines and comments */
>>+ if (line[0] == '\n' || line[0] == '#')
>>+ continue;
>>+
>>+ /* Strip any trailing newline */
>>+ line[strcspn(line, "\n")] = 0;
>>+
>>+ /* Parse the line */
>>+ if (spkgimage_parse_config_line(line, line_num))
>>+ return -EINVAL;
>
>Wouldn't this return -EINVAL; leak memory allocated by the calloc() above?
You are correct. But note that in the normal (non-error) code path, the
structure remains allocated as well, and there is no good place to
free() it, given the available callbacks in struct image_type_params.
So I am relying on the OS to free all memory upon program exit, both in
the error and non-error case. I would think this is reasonable for a
small one-shot utility program, keeps things simple.
If this is not acceptable, I can rework it, but there are quite a few
other spots which would also need to free resources before bailing out.
>[...]
>
>With that fixed:
>
>Reviewed-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
I'll wait to hear back from you before applying this tag.
Regards,
Ralph
More information about the U-Boot
mailing list