[PATCH v1 1/3] perf: Dynamically allocate buffer for event string

Leo Yan posted 3 patches 1 month, 3 weeks ago
[PATCH v1 1/3] perf: Dynamically allocate buffer for event string
Posted by Leo Yan 1 month, 3 weeks ago
Dynamically allocate and free buffer to support event name.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/util/probe-event.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a17c9b8a7a79..cad11d95af4f 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2834,6 +2834,9 @@ static void warn_uprobe_event_compat(struct probe_trace_event *tev)
 	free(buf);
 }
 
+/* Defined in kernel/trace/trace.h */
+#define MAX_EVENT_NAME_LEN	64
+
 /* Set new name from original perf_probe_event and namelist */
 static int probe_trace_event__set_name(struct probe_trace_event *tev,
 				       struct perf_probe_event *pev,
@@ -2841,9 +2844,13 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 				       bool allow_suffix)
 {
 	const char *event, *group;
-	char buf[64];
+	char *buf;
 	int ret;
 
+	buf = malloc(MAX_EVENT_NAME_LEN);
+	if (!buf)
+		return -ENOMEM;
+
 	/* If probe_event or trace_event already have the name, reuse it */
 	if (pev->event && !pev->sdt)
 		event = pev->event;
@@ -2866,17 +2873,19 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 		group = PERFPROBE_GROUP;
 
 	/* Get an unused new event name */
-	ret = get_new_event_name(buf, sizeof(buf), event, namelist,
+	ret = get_new_event_name(buf, MAX_EVENT_NAME_LEN, event, namelist,
 				 tev->point.retprobe, allow_suffix);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	event = buf;
 
 	tev->event = strdup(event);
 	tev->group = strdup(group);
-	if (tev->event == NULL || tev->group == NULL)
-		return -ENOMEM;
+	if (tev->event == NULL || tev->group == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	/*
 	 * Add new event name to namelist if multiprobe event is NOT
@@ -2885,7 +2894,10 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
 	 */
 	if (!multiprobe_event_is_supported())
 		strlist__add(namelist, event);
-	return 0;
+
+out:
+	free(buf);
+	return ret < 0 ? ret : 0;
 }
 
 static int __open_probe_file_and_namelist(bool uprobe,
-- 
2.34.1
Re: [PATCH v1 1/3] perf: Dynamically allocate buffer for event string
Posted by Masami Hiramatsu (Google) 1 month, 2 weeks ago
On Mon,  7 Oct 2024 15:11:14 +0100
Leo Yan <leo.yan@arm.com> wrote:

> Dynamically allocate and free buffer to support event name.

Sorry, but I don't see any benefit from this patch. This looks making
code more complex.

Thank you,

> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  tools/perf/util/probe-event.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a17c9b8a7a79..cad11d95af4f 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2834,6 +2834,9 @@ static void warn_uprobe_event_compat(struct probe_trace_event *tev)
>  	free(buf);
>  }
>  
> +/* Defined in kernel/trace/trace.h */
> +#define MAX_EVENT_NAME_LEN	64
> +
>  /* Set new name from original perf_probe_event and namelist */
>  static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  				       struct perf_probe_event *pev,
> @@ -2841,9 +2844,13 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  				       bool allow_suffix)
>  {
>  	const char *event, *group;
> -	char buf[64];
> +	char *buf;
>  	int ret;
>  
> +	buf = malloc(MAX_EVENT_NAME_LEN);
> +	if (!buf)
> +		return -ENOMEM;
> +
>  	/* If probe_event or trace_event already have the name, reuse it */
>  	if (pev->event && !pev->sdt)
>  		event = pev->event;
> @@ -2866,17 +2873,19 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  		group = PERFPROBE_GROUP;
>  
>  	/* Get an unused new event name */
> -	ret = get_new_event_name(buf, sizeof(buf), event, namelist,
> +	ret = get_new_event_name(buf, MAX_EVENT_NAME_LEN, event, namelist,
>  				 tev->point.retprobe, allow_suffix);
>  	if (ret < 0)
> -		return ret;
> +		goto out;
>  
>  	event = buf;
>  
>  	tev->event = strdup(event);
>  	tev->group = strdup(group);
> -	if (tev->event == NULL || tev->group == NULL)
> -		return -ENOMEM;
> +	if (tev->event == NULL || tev->group == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  
>  	/*
>  	 * Add new event name to namelist if multiprobe event is NOT
> @@ -2885,7 +2894,10 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
>  	 */
>  	if (!multiprobe_event_is_supported())
>  		strlist__add(namelist, event);
> -	return 0;
> +
> +out:
> +	free(buf);
> +	return ret < 0 ? ret : 0;
>  }
>  
>  static int __open_probe_file_and_namelist(bool uprobe,
> -- 
> 2.34.1
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>