On Tue, 14 Sep 2021 16:12:24 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> Some error paths in read_and_parse_cmdlines() may lead to a memory leak.
> Improved the error handling of this internal function to avoid it.
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx>
> ---
> lib/trace-cmd/trace-input.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 60d75d47..9fd7e6e8 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -2999,20 +2999,26 @@ static int read_and_parse_cmdlines(struct tracecmd_input *handle)
> {
> struct tep_handle *pevent = handle->pevent;
> unsigned long long size;
> - char *cmdlines;
> + char *cmdlines = NULL;
> + int ret;
>
> if (handle->file_state >= TRACECMD_FILE_CMD_LINES)
> return 0;
>
> - if (read_data_and_size(handle, &cmdlines, &size) < 0)
> - return -1;
> + ret = read_data_and_size(handle, &cmdlines, &size);
This function only returns an allocated cmdline on success. That was the
point of this function, was that you didn't have to check and free the
return value.
-- Steve
> + if (ret < 0)
> + goto out;
> + if (!size) {
> + handle->file_state = TRACECMD_FILE_CMD_LINES;
> + goto out;
> + }
> cmdlines[size] = 0;
> tep_parse_saved_cmdlines(pevent, cmdlines);
> - free(cmdlines);
> -
> handle->file_state = TRACECMD_FILE_CMD_LINES;
> -
> - return 0;
> + ret = 0;
> +out:
> + free(cmdlines);
> + return ret;
> }
>
> static void extract_trace_clock(struct tracecmd_input *handle, char *line)
|