[PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format

[Marek Vasut]

On 5/8/23 19:50, Ralph Siemsen wrote:
> 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.

The usual fail path handling like:

"
if (there is an error)
   goto exit;
...

exit:
  free(data);
  return ret;
"

does not work here ?