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

[Ralph Siemsen <ralph.siemsen@linaro.org>]

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@mailbox.org>

I'll wait to hear back from you before applying this tag.

Regards,
Ralph