2023-10-12 03:11:53

by Kunwu Chan

Subject: [PATCH] perf tools: Put the function return operation at the end of the function

Function return operations should be performed after resource release.

Fixes: 4e1b9c679fcb ("perf tools: Refactor print_event_desc()")
Signed-off-by: Kunwu.Chan <[email protected]>
---
tools/perf/util/header.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d812e1e371a7..5763f21844b1 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1998,13 +1998,14 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
id++;
}
}
-out:
- free(buf);
- return events;
+
error:
free_event_desc(events);
events = NULL;
- goto out;
+
+out:
+ free(buf);
+ return events;
}

static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
--
2.25.1

2023-10-12 05:03:17

by Adrian Hunter

Subject: Re: [PATCH] perf tools: Put the function return operation at the end of the function

On 12/10/23 06:10, Kunwu.Chan wrote:
> Function return operations should be performed after resource release.
>
> Fixes: 4e1b9c679fcb ("perf tools: Refactor print_event_desc()")
> Signed-off-by: Kunwu.Chan <[email protected]>
> ---
> tools/perf/util/header.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index d812e1e371a7..5763f21844b1 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1998,13 +1998,14 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
> id++;
> }
> }
> -out:
> - free(buf);
> - return events;
> +
> error:
> free_event_desc(events);
> events = NULL;

Always freeing the events is wrong. That path is only for errors.

That would show up if this had been tested.

> - goto out;
> +
> +out:
> + free(buf);
> + return events;
> }
>
> static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,

2023-10-12 06:55:19

by Kunwu Chan

Subject: [PATCH v2] perf tools: Put the function return operation at the end of the function

All error paths go to the Release events process, while under normal
circumstances, go directly to the out process.

Fixes: 4e1b9c679fcb ("perf tools: Refactor print_event_desc()")
Signed-off-by: Kunwu.Chan <[email protected]>
---
tools/perf/util/header.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d812e1e371a7..c2f23381a9c6 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1998,13 +1998,14 @@ static struct evsel *read_event_desc(struct feat_fd *ff)
id++;
}
}
-out:
- free(buf);
- return events;
+goto out;
error:
free_event_desc(events);
events = NULL;
- goto out;
+
+out:
+ free(buf);
+ return events;
}

static int __desc_attr__fprintf(FILE *fp, const char *name, const char *val,
--
2.25.1

Sorry, the wrong patch was sent. The correct patch should be v2 as follows.