[PATCH 0/13] rtla: Code robustness and maintainability improvements

[PATCH 0/13] rtla: Code robustness and maintainability improvements

[Wander Lairson Costa]

This patch series enhances the robustness and maintainability of the
RTLA (Real-Time Linux Analysis) tool through systematic improvements
to error handling, code clarity, and consistency.

The changes strengthen defensive programming practices throughout the
codebase by improving input validation and memory management. Several
new helper functions and macros are introduced to reduce code duplication
and provide safer, more readable alternatives to common operations. The
series also consolidates duplicate logic across modules and adopts
standard conventions where appropriate.

These improvements make the code more resilient and easier to maintain
while preserving existing functionality and behavior.

Wander Lairson Costa (13):
  rtla: Check for memory allocation failures
  rtla: Use strdup() to simplify code
  rtla: Introduce for_each_action() helper
  rtla: Replace atoi() with a robust strtoi()
  rtla: Simplify argument parsing
  rtla: Use strncmp_static() in more places
  rtla: Introduce timerlat_restart() helper
  rtla: Use standard exit codes for result enum
  rtla: Exit if trace output action fails
  rtla: Remove redundant memset after calloc
  rtla: Replace magic number with MAX_PATH
  rtla: Remove unused headers
  rtla: Fix inconsistent state in actions_add_* functions

 tools/tracing/rtla/src/actions.c       | 91 ++++++++++++++++++--------
 tools/tracing/rtla/src/actions.h       |  7 +-
 tools/tracing/rtla/src/osnoise.c       |  6 +-
 tools/tracing/rtla/src/osnoise_hist.c  |  1 -
 tools/tracing/rtla/src/timerlat.c      | 36 +++++++++-
 tools/tracing/rtla/src/timerlat.h      |  9 +++
 tools/tracing/rtla/src/timerlat_hist.c | 32 +++++----
 tools/tracing/rtla/src/timerlat_top.c  | 27 ++++----
 tools/tracing/rtla/src/timerlat_u.c    |  4 +-
 tools/tracing/rtla/src/trace.c         | 23 ++++---
 tools/tracing/rtla/src/utils.c         | 48 +++++++++++---
 tools/tracing/rtla/src/utils.h         | 23 +++++--
 12 files changed, 220 insertions(+), 87 deletions(-)

-- 
2.51.1

[rtla 01/13] rtla: Check for memory allocation failures

[Wander Lairson Costa]

The actions_init() and actions_new() functions did not check the
return value of calloc() and realloc() respectively. In a low
memory situation, this could lead to a NULL pointer dereference.

Add checks for the return value of memory allocation functions
and return an error in case of failure. Update the callers to
handle the error properly.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c       | 26 +++++++++++++++++++++++---
 tools/tracing/rtla/src/actions.h       |  2 +-
 tools/tracing/rtla/src/timerlat_hist.c |  7 +++++--
 tools/tracing/rtla/src/timerlat_top.c  |  7 +++++--
 4 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 8945aee58d511..01648a1425c10 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -11,11 +11,13 @@
 /*
  * actions_init - initialize struct actions
  */
-void
+int
 actions_init(struct actions *self)
 {
 	self->size = action_default_size;
 	self->list = calloc(self->size, sizeof(struct action));
+	if (!self->list)
+		return -1;
 	self->len = 0;
 	self->continue_flag = false;
 
@@ -23,6 +25,7 @@ actions_init(struct actions *self)
 
 	/* This has to be set by the user */
 	self->trace_output_inst = NULL;
+	return 0;
 }
 
 /*
@@ -50,8 +53,13 @@ static struct action *
 actions_new(struct actions *self)
 {
 	if (self->len >= self->size) {
-		self->size *= 2;
-		self->list = realloc(self->list, self->size * sizeof(struct action));
+		const size_t new_size = self->size * 2;
+		void *p = reallocarray(self->list, new_size, sizeof(struct action));
+
+		if (!p)
+			return NULL;
+		self->list = p;
+		self->size = new_size;
 	}
 
 	return &self->list[self->len++];
@@ -65,6 +73,9 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
 {
 	struct action *action = actions_new(self);
 
+	if (!action)
+		return -1;
+
 	self->present[ACTION_TRACE_OUTPUT] = true;
 	action->type = ACTION_TRACE_OUTPUT;
 	action->trace_output = calloc(strlen(trace_output) + 1, sizeof(char));
@@ -83,6 +94,9 @@ actions_add_signal(struct actions *self, int signal, int pid)
 {
 	struct action *action = actions_new(self);
 
+	if (!action)
+		return -1;
+
 	self->present[ACTION_SIGNAL] = true;
 	action->type = ACTION_SIGNAL;
 	action->signal = signal;
@@ -99,6 +113,9 @@ actions_add_shell(struct actions *self, const char *command)
 {
 	struct action *action = actions_new(self);
 
+	if (!action)
+		return -1;
+
 	self->present[ACTION_SHELL] = true;
 	action->type = ACTION_SHELL;
 	action->command = calloc(strlen(command) + 1, sizeof(char));
@@ -117,6 +134,9 @@ actions_add_continue(struct actions *self)
 {
 	struct action *action = actions_new(self);
 
+	if (!action)
+		return -1;
+
 	self->present[ACTION_CONTINUE] = true;
 	action->type = ACTION_CONTINUE;
 
diff --git a/tools/tracing/rtla/src/actions.h b/tools/tracing/rtla/src/actions.h
index a4f9b570775b5..439bcc58ac93a 100644
--- a/tools/tracing/rtla/src/actions.h
+++ b/tools/tracing/rtla/src/actions.h
@@ -42,7 +42,7 @@ struct actions {
 	struct tracefs_instance *trace_output_inst;
 };
 
-void actions_init(struct actions *self);
+int actions_init(struct actions *self);
 void actions_destroy(struct actions *self);
 int actions_add_trace_output(struct actions *self, const char *trace_output);
 int actions_add_signal(struct actions *self, int signal, int pid);
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 606c1688057b2..09a3da3f58630 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -798,8 +798,11 @@ static struct common_params
 	if (!params)
 		exit(1);
 
-	actions_init(&params->common.threshold_actions);
-	actions_init(&params->common.end_actions);
+	if (actions_init(&params->common.threshold_actions) ||
+	    actions_init(&params->common.end_actions)) {
+		err_msg("Error initializing actions");
+		exit(EXIT_FAILURE);
+	}
 
 	/* disabled by default */
 	params->dma_latency = -1;
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index fc479a0dcb597..7679820e72db5 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -556,8 +556,11 @@ static struct common_params
 	if (!params)
 		exit(1);
 
-	actions_init(&params->common.threshold_actions);
-	actions_init(&params->common.end_actions);
+	if (actions_init(&params->common.threshold_actions) ||
+	    actions_init(&params->common.end_actions)) {
+		err_msg("Error initializing actions");
+		exit(EXIT_FAILURE);
+	}
 
 	/* disabled by default */
 	params->dma_latency = -1;
-- 
2.51.1

Re: [rtla 01/13] rtla: Check for memory allocation failures

[Masami Hiramatsu]

On Mon, 17 Nov 2025 15:41:08 -0300
Wander Lairson Costa <wander@redhat.com> wrote:

> The actions_init() and actions_new() functions did not check the
> return value of calloc() and realloc() respectively. In a low
> memory situation, this could lead to a NULL pointer dereference.
> 
> Add checks for the return value of memory allocation functions
> and return an error in case of failure. Update the callers to
> handle the error properly.
> 
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
>  tools/tracing/rtla/src/actions.c       | 26 +++++++++++++++++++++++---
>  tools/tracing/rtla/src/actions.h       |  2 +-
>  tools/tracing/rtla/src/timerlat_hist.c |  7 +++++--
>  tools/tracing/rtla/src/timerlat_top.c  |  7 +++++--
>  4 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> index 8945aee58d511..01648a1425c10 100644
> --- a/tools/tracing/rtla/src/actions.c
> +++ b/tools/tracing/rtla/src/actions.c
> @@ -11,11 +11,13 @@
>  /*
>   * actions_init - initialize struct actions
>   */
> -void
> +int
>  actions_init(struct actions *self)
>  {
>  	self->size = action_default_size;
>  	self->list = calloc(self->size, sizeof(struct action));
> +	if (!self->list)
> +		return -1;

Can you return -ENOMEM?

>  	self->len = 0;
>  	self->continue_flag = false;
>  
> @@ -23,6 +25,7 @@ actions_init(struct actions *self)
>  
>  	/* This has to be set by the user */
>  	self->trace_output_inst = NULL;
> +	return 0;
>  }
>  
>  /*
> @@ -50,8 +53,13 @@ static struct action *
>  actions_new(struct actions *self)
>  {
>  	if (self->len >= self->size) {
> -		self->size *= 2;
> -		self->list = realloc(self->list, self->size * sizeof(struct action));
> +		const size_t new_size = self->size * 2;
> +		void *p = reallocarray(self->list, new_size, sizeof(struct action));
> +
> +		if (!p)
> +			return NULL;
> +		self->list = p;
> +		self->size = new_size;
>  	}
>  
>  	return &self->list[self->len++];
> @@ -65,6 +73,9 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;

I think !action should return -ENOMEM too.

> +
>  	self->present[ACTION_TRACE_OUTPUT] = true;
>  	action->type = ACTION_TRACE_OUTPUT;
>  	action->trace_output = calloc(strlen(trace_output) + 1, sizeof(char));
> @@ -83,6 +94,9 @@ actions_add_signal(struct actions *self, int signal, int pid)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;
> +
>  	self->present[ACTION_SIGNAL] = true;
>  	action->type = ACTION_SIGNAL;
>  	action->signal = signal;
> @@ -99,6 +113,9 @@ actions_add_shell(struct actions *self, const char *command)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;
> +
>  	self->present[ACTION_SHELL] = true;
>  	action->type = ACTION_SHELL;
>  	action->command = calloc(strlen(command) + 1, sizeof(char));
> @@ -117,6 +134,9 @@ actions_add_continue(struct actions *self)
>  {
>  	struct action *action = actions_new(self);
>  
> +	if (!action)
> +		return -1;
> +
>  	self->present[ACTION_CONTINUE] = true;
>  	action->type = ACTION_CONTINUE;
>  

The above same patterns too.

Thank you,



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

Re: [rtla 01/13] rtla: Check for memory allocation failures

[Steven Rostedt]

On Tue, 18 Nov 2025 11:09:46 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Mon, 17 Nov 2025 15:41:08 -0300
> Wander Lairson Costa <wander@redhat.com> wrote:
> 
> > The actions_init() and actions_new() functions did not check the
> > return value of calloc() and realloc() respectively. In a low
> > memory situation, this could lead to a NULL pointer dereference.
> > 
> > Add checks for the return value of memory allocation functions
> > and return an error in case of failure. Update the callers to
> > handle the error properly.
> > 
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > ---
> >  tools/tracing/rtla/src/actions.c       | 26 +++++++++++++++++++++++---
> >  tools/tracing/rtla/src/actions.h       |  2 +-
> >  tools/tracing/rtla/src/timerlat_hist.c |  7 +++++--
> >  tools/tracing/rtla/src/timerlat_top.c  |  7 +++++--
> >  4 files changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> > index 8945aee58d511..01648a1425c10 100644
> > --- a/tools/tracing/rtla/src/actions.c
> > +++ b/tools/tracing/rtla/src/actions.c
> > @@ -11,11 +11,13 @@
> >  /*
> >   * actions_init - initialize struct actions
> >   */
> > -void
> > +int
> >  actions_init(struct actions *self)
> >  {
> >  	self->size = action_default_size;
> >  	self->list = calloc(self->size, sizeof(struct action));
> > +	if (!self->list)
> > +		return -1;  
> 
> Can you return -ENOMEM?

Does it need to? This is user space not the kernel. Errno is already
set by calloc() failing.

-- Steve

Re: [rtla 01/13] rtla: Check for memory allocation failures

[Masami Hiramatsu]

On Mon, 17 Nov 2025 22:06:15 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 18 Nov 2025 11:09:46 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > On Mon, 17 Nov 2025 15:41:08 -0300
> > Wander Lairson Costa <wander@redhat.com> wrote:
> > 
> > > The actions_init() and actions_new() functions did not check the
> > > return value of calloc() and realloc() respectively. In a low
> > > memory situation, this could lead to a NULL pointer dereference.
> > > 
> > > Add checks for the return value of memory allocation functions
> > > and return an error in case of failure. Update the callers to
> > > handle the error properly.
> > > 
> > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > > ---
> > >  tools/tracing/rtla/src/actions.c       | 26 +++++++++++++++++++++++---
> > >  tools/tracing/rtla/src/actions.h       |  2 +-
> > >  tools/tracing/rtla/src/timerlat_hist.c |  7 +++++--
> > >  tools/tracing/rtla/src/timerlat_top.c  |  7 +++++--
> > >  4 files changed, 34 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> > > index 8945aee58d511..01648a1425c10 100644
> > > --- a/tools/tracing/rtla/src/actions.c
> > > +++ b/tools/tracing/rtla/src/actions.c
> > > @@ -11,11 +11,13 @@
> > >  /*
> > >   * actions_init - initialize struct actions
> > >   */
> > > -void
> > > +int
> > >  actions_init(struct actions *self)
> > >  {
> > >  	self->size = action_default_size;
> > >  	self->list = calloc(self->size, sizeof(struct action));
> > > +	if (!self->list)
> > > +		return -1;  
> > 
> > Can you return -ENOMEM?
> 
> Does it need to? This is user space not the kernel. Errno is already
> set by calloc() failing.

Ah, indeed! I agree to just return -1.

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

Thank you,

> 
> -- Steve
> 


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

Re: [rtla 01/13] rtla: Check for memory allocation failures

[Costa Shulyupin]

On Mon, 17 Nov 2025 at 20:54, Wander Lairson Costa <wander@redhat.com> wrote:
> Add checks for the return value of memory allocation functions
> and return an error in case of failure. Update the callers to
> handle the error properly.

Would you like to consider using fatal("Out of memory") instead of
returning an error code?
Anyway there is no work around for out of memory.

Costa

Re: [rtla 01/13] rtla: Check for memory allocation failures

[Wander Lairson Costa]

On Fri, Nov 28, 2025 at 10:30 AM Costa Shulyupin <costa.shul@redhat.com> wrote:
>
> On Mon, 17 Nov 2025 at 20:54, Wander Lairson Costa <wander@redhat.com> wrote:
> > Add checks for the return value of memory allocation functions
> > and return an error in case of failure. Update the callers to
> > handle the error properly.
>
> Would you like to consider using fatal("Out of memory") instead of
> returning an error code?
> Anyway there is no work around for out of memory.
>

Good idea. I am going to update the patches.

> Costa
>

[rtla 02/13] rtla: Use strdup() to simplify code

[Wander Lairson Costa]

The actions_add_trace_output() and actions_add_shell() functions were
using calloc() followed by strcpy() to allocate and copy a string.
This can be simplified by using strdup(), which allocates memory and
copies the string in a single step.

Replace the calloc() and strcpy() calls with strdup(), making the
code more concise and readable.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 01648a1425c10..696dd1ed98d9a 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -78,10 +78,9 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
 
 	self->present[ACTION_TRACE_OUTPUT] = true;
 	action->type = ACTION_TRACE_OUTPUT;
-	action->trace_output = calloc(strlen(trace_output) + 1, sizeof(char));
+	action->trace_output = strdup(trace_output);
 	if (!action->trace_output)
 		return -1;
-	strcpy(action->trace_output, trace_output);
 
 	return 0;
 }
@@ -118,10 +117,9 @@ actions_add_shell(struct actions *self, const char *command)
 
 	self->present[ACTION_SHELL] = true;
 	action->type = ACTION_SHELL;
-	action->command = calloc(strlen(command) + 1, sizeof(char));
+	action->command = strdup(command);
 	if (!action->command)
 		return -1;
-	strcpy(action->command, command);
 
 	return 0;
 }
-- 
2.51.1

[rtla 03/13] rtla: Introduce for_each_action() helper

[Wander Lairson Costa]

The for loop to iterate over the list of actions is used in
more than one place. To avoid code duplication and improve
readability, introduce a for_each_action() helper macro.

Replace the open-coded for loops with the new helper.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c | 6 ++++--
 tools/tracing/rtla/src/actions.h | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 696dd1ed98d9a..efa17290926da 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -35,7 +35,9 @@ void
 actions_destroy(struct actions *self)
 {
 	/* Free any action-specific data */
-	for (struct action *action = self->list; action < self->list + self->len; action++) {
+	struct action *action;
+
+	for_each_action(self, action) {
 		if (action->type == ACTION_SHELL)
 			free(action->command);
 		if (action->type == ACTION_TRACE_OUTPUT)
@@ -241,7 +243,7 @@ actions_perform(struct actions *self)
 	int pid, retval;
 	const struct action *action;
 
-	for (action = self->list; action < self->list + self->len; action++) {
+	for_each_action(self, action) {
 		switch (action->type) {
 		case ACTION_TRACE_OUTPUT:
 			retval = save_trace_to_file(self->trace_output_inst, action->trace_output);
diff --git a/tools/tracing/rtla/src/actions.h b/tools/tracing/rtla/src/actions.h
index 439bcc58ac93a..9b9df57a0cb34 100644
--- a/tools/tracing/rtla/src/actions.h
+++ b/tools/tracing/rtla/src/actions.h
@@ -42,6 +42,11 @@ struct actions {
 	struct tracefs_instance *trace_output_inst;
 };
 
+#define for_each_action(actions, action)			\
+	for ((action) = (actions)->list;			\
+	     (action) < (actions)->list + (actions)->len;	\
+	     (action)++)
+
 int actions_init(struct actions *self);
 void actions_destroy(struct actions *self);
 int actions_add_trace_output(struct actions *self, const char *trace_output);
-- 
2.51.1

[rtla 04/13] rtla: Replace atoi() with a robust strtoi()

[Wander Lairson Costa]

The atoi() function does not perform error checking, which can lead to
undefined behavior when parsing invalid or out-of-range strings. This
can cause issues when parsing user-provided numerical inputs, such as
signal numbers, PIDs, or CPU lists.

To address this, introduce a new strtoi() helper function that safely
converts a string to an integer. This function validates the input and
checks for overflows, returning a boolean to indicate success or failure.

Replace all calls to atoi() with the new strtoi() function and add
proper error handling to make the parsing more robust and prevent
potential issues.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c |  6 +++--
 tools/tracing/rtla/src/utils.c   | 40 ++++++++++++++++++++++++++++----
 tools/tracing/rtla/src/utils.h   |  2 ++
 3 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index efa17290926da..e23d4f1c5a592 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
 		/* Takes two arguments, num (signal) and pid */
 		while (token != NULL) {
 			if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
-				signal = atoi(token + 4);
+				if(!strtoi(token + 4, &signal))
+					return -1;
 			} else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
 				if (strncmp(token + 4, "parent", 7) == 0)
 					pid = -1;
 				else
-					pid = atoi(token + 4);
+					if (!strtoi(token + 4, &pid))
+						return -1;
 			} else {
 				/* Invalid argument */
 				return -1;
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index d6ab15dcb4907..4cb765b94feec 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -17,6 +17,7 @@
 #include <fcntl.h>
 #include <sched.h>
 #include <stdio.h>
+#include <limits.h>
 
 #include "utils.h"
 
@@ -112,16 +113,18 @@ int parse_cpu_set(char *cpu_list, cpu_set_t *set)
 	nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
 
 	for (p = cpu_list; *p; ) {
-		cpu = atoi(p);
-		if (cpu < 0 || (!cpu && *p != '0') || cpu >= nr_cpus)
+		if (!strtoi(p, &cpu))
+			goto err;
+		if (cpu < 0 || cpu >= nr_cpus)
 			goto err;
 
 		while (isdigit(*p))
 			p++;
 		if (*p == '-') {
 			p++;
-			end_cpu = atoi(p);
-			if (end_cpu < cpu || (!end_cpu && *p != '0') || end_cpu >= nr_cpus)
+			if (!strtoi(p, &end_cpu))
+				goto err;
+			if (end_cpu < cpu || end_cpu >= nr_cpus)
 				goto err;
 			while (isdigit(*p))
 				p++;
@@ -322,6 +325,7 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
 	struct dirent *proc_entry;
 	DIR *procfs;
 	int retval;
+	int pid;
 
 	if (strlen(comm_prefix) >= MAX_PATH) {
 		err_msg("Command prefix is too long: %d < strlen(%s)\n",
@@ -341,8 +345,12 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
 		if (!retval)
 			continue;
 
+		if (!strtoi(proc_entry->d_name, &pid)) {
+			err_msg("'%s' is not a valid pid", proc_entry->d_name);
+			goto out_err;
+		}
 		/* procfs_is_workload_pid confirmed it is a pid */
-		retval = __set_sched_attr(atoi(proc_entry->d_name), attr);
+		retval = __set_sched_attr(pid, attr);
 		if (retval) {
 			err_msg("Error setting sched attributes for pid:%s\n", proc_entry->d_name);
 			goto out_err;
@@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
 
 	return 1;
 }
+
+/*
+ * strtoi - convert string to integer with error checking
+ *
+ * Returns true on success, false if conversion fails or result is out of int range.
+ */
+bool strtoi(const char *s, int *res)
+{
+	char *end_ptr;
+	long lres;
+
+	if (!*s)
+		return false;
+
+	errno = 0;
+	lres = strtol(s, &end_ptr, 0);
+	if (errno || *end_ptr || lres > INT_MAX || lres < INT_MIN)
+		return false;
+
+	*res = (int) lres;
+	return true;
+}
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index a2a6f89f342d0..160491f5de91c 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -3,6 +3,7 @@
 #include <stdint.h>
 #include <time.h>
 #include <sched.h>
+#include <stdbool.h>
 
 /*
  * '18446744073709551615\0'
@@ -80,6 +81,7 @@ static inline int set_deepest_cpu_idle_state(unsigned int cpu, unsigned int stat
 static inline int have_libcpupower_support(void) { return 0; }
 #endif /* HAVE_LIBCPUPOWER_SUPPORT */
 int auto_house_keeping(cpu_set_t *monitored_cpus);
+bool strtoi(const char *s, int *res);
 
 #define ns_to_usf(x) (((double)x/1000))
 #define ns_to_per(total, part) ((part * 100) / (double)total)
-- 
2.51.1

Re: [rtla 04/13] rtla: Replace atoi() with a robust strtoi()

[Crystal Wood]

On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:

> 
> diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> index efa17290926da..e23d4f1c5a592 100644
> --- a/tools/tracing/rtla/src/actions.c
> +++ b/tools/tracing/rtla/src/actions.c
> @@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
>  		/* Takes two arguments, num (signal) and pid */
>  		while (token != NULL) {
>  			if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
> -				signal = atoi(token + 4);
> +				if(!strtoi(token + 4, &signal))
> +					return -1;

if (

>  			} else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
>  				if (strncmp(token + 4, "parent", 7) == 0)
>  					pid = -1;
>  				else
> -					pid = atoi(token + 4);
> +					if (!strtoi(token + 4, &pid))
> +						return -1;

else if (

Please run the patches through checkpatch.pl

> @@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
>  
>  	return 1;
>  }
> +
> +/*
> + * strtoi - convert string to integer with error checking
> + *
> + * Returns true on success, false if conversion fails or result is out of int range.
> + */
> +bool strtoi(const char *s, int *res)

Could use __attribute__((__warn_unused_result__)) like kstrtoint().

BTW, it's pretty annoying that we need to reinvent the wheel on all this
stuff just because it's userspace.  From some of the other tools it
looks like we can at least include basic kernel headers like compiler.h;
maybe we should have a tools/-wide common util area as well?  Even
better if some of the code can be shared with the kernel itself.

Not saying that should in any way be a blocker for these patches, just
something to think about.


-Crystal

Re: [rtla 04/13] rtla: Replace atoi() with a robust strtoi()

[Wander Lairson Costa]

On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
>
> On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> >
> > diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> > index efa17290926da..e23d4f1c5a592 100644
> > --- a/tools/tracing/rtla/src/actions.c
> > +++ b/tools/tracing/rtla/src/actions.c
> > @@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
> >               /* Takes two arguments, num (signal) and pid */
> >               while (token != NULL) {
> >                       if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
> > -                             signal = atoi(token + 4);
> > +                             if(!strtoi(token + 4, &signal))
> > +                                     return -1;
>
> if (
>
> >                       } else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
> >                               if (strncmp(token + 4, "parent", 7) == 0)
> >                                       pid = -1;
> >                               else
> > -                                     pid = atoi(token + 4);
> > +                                     if (!strtoi(token + 4, &pid))
> > +                                             return -1;
>
> else if (
>
> Please run the patches through checkpatch.pl
>

Good catch, thanks.

> > @@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
> >
> >       return 1;
> >  }
> > +
> > +/*
> > + * strtoi - convert string to integer with error checking
> > + *
> > + * Returns true on success, false if conversion fails or result is out of int range.
> > + */
> > +bool strtoi(const char *s, int *res)
>
> Could use __attribute__((__warn_unused_result__)) like kstrtoint().
>

Sure, I will do it in v2.

> BTW, it's pretty annoying that we need to reinvent the wheel on all this
> stuff just because it's userspace.  From some of the other tools it
> looks like we can at least include basic kernel headers like compiler.h;
> maybe we should have a tools/-wide common util area as well?  Even
> better if some of the code can be shared with the kernel itself.
>
> Not saying that should in any way be a blocker for these patches, just
> something to think about.
>

I thought the same thing some time ago.

>
> -Crystal
>

Re: [rtla 04/13] rtla: Replace atoi() with a robust strtoi()

[Costa Shulyupin]

On Mon, 17 Nov 2025 at 20:55, Wander Lairson Costa <wander@redhat.com> wrote:
> To address this, introduce a new strtoi() helper function that safely
> converts a string to an integer. This function validates the input and
> checks for overflows, returning a boolean to indicate success or failure.

Why not use sscanf() for this purpose instead of adding a new utility function?

Also, using a boolean to return success or failure does not conform to
POSIX standards and is confusing in Linux/POSIX code.

Costa

Re: [rtla 04/13] rtla: Replace atoi() with a robust strtoi()

[Wander Lairson Costa]

On Tue, Nov 25, 2025 at 10:35:39AM +0200, Costa Shulyupin wrote:
> On Mon, 17 Nov 2025 at 20:55, Wander Lairson Costa <wander@redhat.com> wrote:
> > To address this, introduce a new strtoi() helper function that safely
> > converts a string to an integer. This function validates the input and
> > checks for overflows, returning a boolean to indicate success or failure.
> 
> Why not use sscanf() for this purpose instead of adding a new utility function?
> 

The strtoi implementation properly detects:

1. Empty strings - via the !*s check
2. Conversion errors - via errno from strtol
3. Trailing garbage - via *end_ptr check ensuring entire string was consumed
4. Integer overflow/underflow - via explicit lres > INT_MAX || lres < INT_MIN
   bounds checking

sscanf has the following limitations:

1. Trailing garbage is silently ignored

   int val;
   sscanf("123abc", "%d", &val);  /* Returns 1 (success), val=123, "abc" ignored */

   While you could use "%d%n" with character count checking, this becomes
   cumbersome and defeats the purpose of simplification.

2. Integer overflow has undefined behavior

   sscanf with %d doesn't guarantee overflow detection and may silently wrap
   values (e.g., 2147483648 -> -2147483648). There's no standard way to detect
   this has occurred.

3. No detailed error reporting (this is minor, though)

   sscanf only returns match count, not error type. You cannot distinguish
   "bad format" from "overflow" from "trailing garbage".

> Also, using a boolean to return success or failure does not conform to
> POSIX standards and is confusing in Linux/POSIX code.
> 

Ok, I will change it.

> Costa
> 

[rtla 05/13] rtla: Simplify argument parsing

[Wander Lairson Costa]

The actions_parse() function uses open-coded logic to extract arguments
from a string. This includes manual length checks and strncmp() calls,
which can be verbose and error-prone.

To simplify and improve the robustness of argument parsing, introduce a
new extract_arg() helper macro. This macro extracts the value from a
"key=value" pair, making the code more concise and readable.

Also, introduce STRING_LENGTH() and strncmp_static() macros to
perform compile-time calculations of string lengths and safer string
comparisons.

Refactor actions_parse() to use these new helpers, resulting in
cleaner and more maintainable code.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c | 37 ++++++++++++++++++++++----------
 tools/tracing/rtla/src/utils.h   | 14 ++++++++++--
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index e23d4f1c5a592..2a01ece78454c 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -143,15 +143,30 @@ actions_add_continue(struct actions *self)
 	return 0;
 }
 
+/*
+ * extract_arg - extract argument value from option token
+ * @token: option token (e.g., "file=trace.txt")
+ * @opt: option name to match (e.g., "file")
+ *
+ * Returns pointer to argument value after "=" if token matches "opt=",
+ * otherwise returns NULL.
+ */
+#define extract_arg(token, opt) (				\
+	strlen(token) > STRING_LENGTH(opt "=") &&		\
+	!strncmp_static(token, opt "=")				\
+		? (token) + STRING_LENGTH(opt "=") : NULL )
+
 /*
  * actions_parse - add an action based on text specification
  */
 int
 actions_parse(struct actions *self, const char *trigger, const char *tracefn)
 {
+
 	enum action_type type = ACTION_NONE;
 	const char *token;
 	char trigger_c[strlen(trigger) + 1];
+	const char *arg_value;
 
 	/* For ACTION_SIGNAL */
 	int signal = 0, pid = 0;
@@ -182,12 +197,10 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
 		if (token == NULL)
 			trace_output = tracefn;
 		else {
-			if (strlen(token) > 5 && strncmp(token, "file=", 5) == 0) {
-				trace_output = token + 5;
-			} else {
+			trace_output = extract_arg(token, "file");
+			if (!trace_output)
 				/* Invalid argument */
 				return -1;
-			}
 
 			token = strtok(NULL, ",");
 			if (token != NULL)
@@ -198,14 +211,15 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
 	case ACTION_SIGNAL:
 		/* Takes two arguments, num (signal) and pid */
 		while (token != NULL) {
-			if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
-				if(!strtoi(token + 4, &signal))
+			arg_value = extract_arg(token, "num");
+			if (arg_value) {
+				if (!strtoi(arg_value, &signal))
 					return -1;
-			} else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
-				if (strncmp(token + 4, "parent", 7) == 0)
+			} else if ((arg_value = extract_arg(token, "pid"))) {
+				if (strncmp_static(arg_value, "parent") == 0)
 					pid = -1;
 				else
-					if (!strtoi(token + 4, &pid))
+					if (!strtoi(arg_value, &pid))
 						return -1;
 			} else {
 				/* Invalid argument */
@@ -223,8 +237,9 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
 	case ACTION_SHELL:
 		if (token == NULL)
 			return -1;
-		if (strlen(token) > 8 && strncmp(token, "command=", 8) == 0)
-			return actions_add_shell(self, token + 8);
+		arg_value = extract_arg(token, "command");
+		if (arg_value)
+			return actions_add_shell(self, arg_value);
 		return -1;
 	case ACTION_CONTINUE:
 		/* Takes no argument */
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index 160491f5de91c..f7ff548f7fba7 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -13,8 +13,18 @@
 #define MAX_NICE		20
 #define MIN_NICE		-19
 
-#define container_of(ptr, type, member)({			\
-	const typeof(((type *)0)->member) *__mptr = (ptr);	\
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
+#endif
+
+/* Calculate string length at compile time (excluding null terminator) */
+#define STRING_LENGTH(s) (ARRAY_SIZE(s) - sizeof(*(s)))
+
+/* Compare string with static string, length determined at compile time */
+#define strncmp_static(s1, s2) strncmp(s1, s2, STRING_LENGTH(s2))
+
+#define container_of(ptr, type, member)({				\
+	const typeof(((type *)0)->member) *__mptr = (ptr);		\
 	(type *)((char *)__mptr - offsetof(type, member)) ; })
 
 extern int config_debug;
-- 
2.51.1

Re: [rtla 05/13] rtla: Simplify argument parsing

[Crystal Wood]

On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:

>  
> +/*
> + * extract_arg - extract argument value from option token
> + * @token: option token (e.g., "file=trace.txt")
> + * @opt: option name to match (e.g., "file")
> + *
> + * Returns pointer to argument value after "=" if token matches "opt=",
> + * otherwise returns NULL.
> + */
> +#define extract_arg(token, opt) (				\
> +	strlen(token) > STRING_LENGTH(opt "=") &&		\
> +	!strncmp_static(token, opt "=")				\
> +		? (token) + STRING_LENGTH(opt "=") : NULL )

This could be implemented as a function (albeit without the
concatenation trick)... but if it must be a macro, at least encase it
with ({ }) so it behaves more like a function.

> +
>  /*
>   * actions_parse - add an action based on text specification
>   */
>  int
>  actions_parse(struct actions *self, const char *trigger, const char *tracefn)
>  {
> +
>  	enum action_type type = ACTION_NONE;

Why this blank line?


> diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> index 160491f5de91c..f7ff548f7fba7 100644
> --- a/tools/tracing/rtla/src/utils.h
> +++ b/tools/tracing/rtla/src/utils.h
> @@ -13,8 +13,18 @@
>  #define MAX_NICE		20
>  #define MIN_NICE		-19
>  
> -#define container_of(ptr, type, member)({			\
> -	const typeof(((type *)0)->member) *__mptr = (ptr);	\
[snip]
> +#define container_of(ptr, type, member)({				\
> +	const typeof(((type *)0)->member) *__mptr = (ptr);		\
>  	(type *)((char *)__mptr - offsetof(type, member)) ; })

It's easier to review patches when they don't make unrelated aesthetic
changes...

-Crystal

Re: [rtla 05/13] rtla: Simplify argument parsing

[Wander Lairson Costa]

On Mon, Nov 24, 2025 at 06:46:33PM -0600, Crystal Wood wrote:
> On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
> 
> >  
> > +/*
> > + * extract_arg - extract argument value from option token
> > + * @token: option token (e.g., "file=trace.txt")
> > + * @opt: option name to match (e.g., "file")
> > + *
> > + * Returns pointer to argument value after "=" if token matches "opt=",
> > + * otherwise returns NULL.
> > + */
> > +#define extract_arg(token, opt) (				\
> > +	strlen(token) > STRING_LENGTH(opt "=") &&		\
> > +	!strncmp_static(token, opt "=")				\
> > +		? (token) + STRING_LENGTH(opt "=") : NULL )
> 
> This could be implemented as a function (albeit without the
> concatenation trick)... but if it must be a macro, at least encase it
> with ({ }) so it behaves more like a function.
> 
> > +
> >  /*
> >   * actions_parse - add an action based on text specification
> >   */
> >  int
> >  actions_parse(struct actions *self, const char *trigger, const char *tracefn)
> >  {
> > +
> >  	enum action_type type = ACTION_NONE;
> 
> Why this blank line?
> 

Must be a typo during the rebase process. I will remove it.

> 
> > diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> > index 160491f5de91c..f7ff548f7fba7 100644
> > --- a/tools/tracing/rtla/src/utils.h
> > +++ b/tools/tracing/rtla/src/utils.h
> > @@ -13,8 +13,18 @@
> >  #define MAX_NICE		20
> >  #define MIN_NICE		-19
> >  
> > -#define container_of(ptr, type, member)({			\
> > -	const typeof(((type *)0)->member) *__mptr = (ptr);	\
> [snip]
> > +#define container_of(ptr, type, member)({				\
> > +	const typeof(((type *)0)->member) *__mptr = (ptr);		\
> >  	(type *)((char *)__mptr - offsetof(type, member)) ; })
> 
> It's easier to review patches when they don't make unrelated aesthetic
> changes...
> 

It is just git messing up the diff. No actual change.

> -Crystal
> 

Re: [rtla 05/13] rtla: Simplify argument parsing

[Crystal Wood]

On Tue, 2025-11-25 at 10:45 -0300, Wander Lairson Costa wrote:
> On Mon, Nov 24, 2025 at 06:46:33PM -0600, Crystal Wood wrote:
> > On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
> > > diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> > > index 160491f5de91c..f7ff548f7fba7 100644
> > > --- a/tools/tracing/rtla/src/utils.h
> > > +++ b/tools/tracing/rtla/src/utils.h
> > > @@ -13,8 +13,18 @@
> > >  #define MAX_NICE		20
> > >  #define MIN_NICE		-19
> > >  
> > > -#define container_of(ptr, type, member)({			\
> > > -	const typeof(((type *)0)->member) *__mptr = (ptr);	\
> > [snip]
> > > +#define container_of(ptr, type, member)({				\
> > > +	const typeof(((type *)0)->member) *__mptr = (ptr);		\
> > >  	(type *)((char *)__mptr - offsetof(type, member)) ; })
> > 
> > It's easier to review patches when they don't make unrelated aesthetic
> > changes...
> > 
> 
> It is just git messing up the diff. No actual change.

The change was to the position of the backslashes.  Not a big deal, just
something that's helpful to avoid to get a cleaner diff.

-Crystal

[rtla 06/13] rtla: Use strncmp_static() in more places

[Wander Lairson Costa]

The recently introduced strncmp_static() helper provides a safer way
to compare strings with static strings by determining the length at
compile time.

Replace several open-coded strncmp() calls with strncmp_static() to
improve code readability and robustness. This change affects the
parsing of command-line arguments and environment variables.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/osnoise.c  | 2 +-
 tools/tracing/rtla/src/timerlat.c | 4 ++--
 tools/tracing/rtla/src/trace.c    | 2 +-
 tools/tracing/rtla/src/utils.c    | 8 ++++----
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 312c511fa0044..5075e0f485c77 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -1229,7 +1229,7 @@ int osnoise_main(int argc, char *argv[])
 
 	if ((strcmp(argv[1], "-h") == 0) || (strcmp(argv[1], "--help") == 0)) {
 		osnoise_usage(0);
-	} else if (strncmp(argv[1], "-", 1) == 0) {
+	} else if (strncmp_static(argv[1], "-") == 0) {
 		/* the user skipped the tool, call the default one */
 		run_tool(&osnoise_top_ops, argc, argv);
 		exit(0);
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index b692128741279..56e0b8af041d7 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -34,7 +34,7 @@ timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params)
 	 * Try to enable BPF, unless disabled explicitly.
 	 * If BPF enablement fails, fall back to tracefs mode.
 	 */
-	if (getenv("RTLA_NO_BPF") && strncmp(getenv("RTLA_NO_BPF"), "1", 2) == 0) {
+	if (getenv("RTLA_NO_BPF") && strncmp_static(getenv("RTLA_NO_BPF"), "1") == 0) {
 		debug_msg("RTLA_NO_BPF set, disabling BPF\n");
 		params->mode = TRACING_MODE_TRACEFS;
 	} else if (!tep_find_event_by_name(tool->trace.tep, "osnoise", "timerlat_sample")) {
@@ -275,7 +275,7 @@ int timerlat_main(int argc, char *argv[])
 
 	if ((strcmp(argv[1], "-h") == 0) || (strcmp(argv[1], "--help") == 0)) {
 		timerlat_usage(0);
-	} else if (strncmp(argv[1], "-", 1) == 0) {
+	} else if (strncmp_static(argv[1], "-") == 0) {
 		/* the user skipped the tool, call the default one */
 		run_tool(&timerlat_top_ops, argc, argv);
 		exit(0);
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 69cbc48d53d3a..813f4368f104b 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -372,7 +372,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
 		return;
 
 	/* is this a hist: trigger? */
-	retval = strncmp(tevent->trigger, "hist:", strlen("hist:"));
+	retval = strncmp_static(tevent->trigger, "hist:");
 	if (retval)
 		return;
 
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index 4cb765b94feec..f13d00d7b6bfe 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -196,15 +196,15 @@ long parse_ns_duration(char *val)
 	t = strtol(val, &end, 10);
 
 	if (end) {
-		if (!strncmp(end, "ns", 2)) {
+		if (!strncmp_static(end, "ns")) {
 			return t;
-		} else if (!strncmp(end, "us", 2)) {
+		} else if (!strncmp_static(end, "us")) {
 			t *= 1000;
 			return t;
-		} else if (!strncmp(end, "ms", 2)) {
+		} else if (!strncmp_static(end, "ms")) {
 			t *= 1000 * 1000;
 			return t;
-		} else if (!strncmp(end, "s", 1)) {
+		} else if (!strncmp_static(end, "s")) {
 			t *= 1000 * 1000 * 1000;
 			return t;
 		}
-- 
2.51.1

[rtla 07/13] rtla: Introduce timerlat_restart() helper

[Wander Lairson Costa]

The timerlat_hist and timerlat_top commands duplicate the logic for
handling threshold actions. When a threshold is reached, both commands
stop the trace, perform actions, and restart the trace if configured to
continue.

Create a new helper function, timerlat_restart(), to centralize this
shared logic and avoid code duplication. This function now handles the
threshold actions and restarts the necessary trace instances.

Refactor timerlat_hist_main() and the timerlat_top main loops to call
the new helper. This makes the code cleaner and more maintainable.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/timerlat.c      | 31 ++++++++++++++++++++++++++
 tools/tracing/rtla/src/timerlat.h      |  9 ++++++++
 tools/tracing/rtla/src/timerlat_hist.c | 19 ++++++++--------
 tools/tracing/rtla/src/timerlat_top.c  | 19 ++++++++--------
 4 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 56e0b8af041d7..50c7eb00fd6b7 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -22,6 +22,37 @@
 
 static int dma_latency_fd = -1;
 
+/**
+ * timerlat_restart - handle threshold actions and optionally restart tracing
+ * @tool: pointer to the osnoise_tool instance containing trace contexts
+ * @params: timerlat parameters with threshold action configuration
+ *
+ * Return:
+ *   RESTART_OK - Actions executed successfully and tracing restarted
+ *   RESTART_STOP - Actions executed but 'continue' flag not set, stop tracing
+ *   RESTART_ERROR - Failed to restart tracing after executing actions
+ */
+enum restart_result
+timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params)
+{
+	actions_perform(&params->common.threshold_actions);
+
+	if (!params->common.threshold_actions.continue_flag)
+		/* continue flag not set, break */
+		return RESTART_STOP;
+
+	/* continue action reached, re-enable tracing */
+	if (tool->record && trace_instance_start(&tool->record->trace))
+		goto err;
+	if (tool->aa && trace_instance_start(&tool->aa->trace))
+		goto err;
+	return RESTART_OK;
+
+err:
+	err_msg("Error restarting trace\n");
+	return RESTART_ERROR;
+}
+
 /*
  * timerlat_apply_config - apply common configs to the initialized tool
  */
diff --git a/tools/tracing/rtla/src/timerlat.h b/tools/tracing/rtla/src/timerlat.h
index fd6065f48bb7f..47a34bb443fa0 100644
--- a/tools/tracing/rtla/src/timerlat.h
+++ b/tools/tracing/rtla/src/timerlat.h
@@ -31,6 +31,15 @@ struct timerlat_params {
 
 #define to_timerlat_params(ptr) container_of(ptr, struct timerlat_params, common)
 
+enum restart_result {
+	RESTART_OK,
+	RESTART_STOP,
+	RESTART_ERROR = -1,
+};
+
+enum restart_result
+timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params);
+
 int timerlat_apply_config(struct osnoise_tool *tool, struct timerlat_params *params);
 int timerlat_main(int argc, char *argv[]);
 int timerlat_enable(struct osnoise_tool *tool);
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 09a3da3f58630..f14fc56c5b4a5 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -1165,18 +1165,19 @@ static int timerlat_hist_bpf_main_loop(struct osnoise_tool *tool)
 
 		if (!stop_tracing) {
 			/* Threshold overflow, perform actions on threshold */
-			actions_perform(&params->common.threshold_actions);
+			enum restart_result result;
 
-			if (!params->common.threshold_actions.continue_flag)
-				/* continue flag not set, break */
+			result = timerlat_restart(tool, params);
+			if (result == RESTART_STOP)
 				break;
 
-			/* continue action reached, re-enable tracing */
-			if (tool->record)
-				trace_instance_start(&tool->record->trace);
-			if (tool->aa)
-				trace_instance_start(&tool->aa->trace);
-			timerlat_bpf_restart_tracing();
+			if (result == RESTART_ERROR)
+				return -1;
+
+			if (timerlat_bpf_restart_tracing()) {
+				err_msg("Error restarting BPF trace\n");
+				return -1;
+			}
 		}
 	}
 	timerlat_bpf_detach();
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index 7679820e72db5..d831a9e1818f4 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -921,18 +921,19 @@ timerlat_top_bpf_main_loop(struct osnoise_tool *tool)
 
 		if (wait_retval == 1) {
 			/* Stopping requested by tracer */
-			actions_perform(&params->common.threshold_actions);
+			enum restart_result result;
 
-			if (!params->common.threshold_actions.continue_flag)
-				/* continue flag not set, break */
+			result = timerlat_restart(tool, params);
+			if (result == RESTART_STOP)
 				break;
 
-			/* continue action reached, re-enable tracing */
-			if (tool->record)
-				trace_instance_start(&tool->record->trace);
-			if (tool->aa)
-				trace_instance_start(&tool->aa->trace);
-			timerlat_bpf_restart_tracing();
+			if (result == RESTART_ERROR)
+				return -1;
+
+			if (timerlat_bpf_restart_tracing()) {
+				err_msg("Error restarting BPF trace\n");
+				return -1;
+			}
 		}
 
 		/* is there still any user-threads ? */
-- 
2.51.1

Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper

[Crystal Wood]

On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:

> +enum restart_result
> +timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params)
> +{
> +	actions_perform(&params->common.threshold_actions);
> +
> +	if (!params->common.threshold_actions.continue_flag)
> +		/* continue flag not set, break */
> +		return RESTART_STOP;
> +
> +	/* continue action reached, re-enable tracing */
> +	if (tool->record && trace_instance_start(&tool->record->trace))
> +		goto err;
> +	if (tool->aa && trace_instance_start(&tool->aa->trace))
> +		goto err;
> +	return RESTART_OK;
> +
> +err:
> +	err_msg("Error restarting trace\n");
> +	return RESTART_ERROR;
> +}

The non-BPF functions in common.c have the same logic and should also
call this.  This isn't timerlat-specific.


> diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
> index 09a3da3f58630..f14fc56c5b4a5 100644
> --- a/tools/tracing/rtla/src/timerlat_hist.c
> +++ b/tools/tracing/rtla/src/timerlat_hist.c
> @@ -1165,18 +1165,19 @@ static int timerlat_hist_bpf_main_loop(struct osnoise_tool *tool)
>  
>  		if (!stop_tracing) {
>  			/* Threshold overflow, perform actions on threshold */
> -			actions_perform(&params->common.threshold_actions);
> +			enum restart_result result;
>  
> -			if (!params->common.threshold_actions.continue_flag)
> -				/* continue flag not set, break */
> +			result = timerlat_restart(tool, params);
> +			if (result == RESTART_STOP)
>  				break;
>  
> -			/* continue action reached, re-enable tracing */
> -			if (tool->record)
> -				trace_instance_start(&tool->record->trace);
> -			if (tool->aa)
> -				trace_instance_start(&tool->aa->trace);
> -			timerlat_bpf_restart_tracing();
> +			if (result == RESTART_ERROR)
> +				return -1;

Does it matter that we're not detaching on an error here?  Is this
something that gets cleaned up automatically (and if so, why do we ever
need to do it explicitly)?

> +
> +			if (timerlat_bpf_restart_tracing()) {
> +				err_msg("Error restarting BPF trace\n");
> +				return -1;
> +			}

[insert rant about not being able to use exceptions in userspace code in
the year 2025]

-Crystal

Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper

[Wander Lairson Costa]

On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
>
> On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> > +enum restart_result
> > +timerlat_restart(const struct osnoise_tool *tool, struct timerlat_params *params)
> > +{
> > +     actions_perform(&params->common.threshold_actions);
> > +
> > +     if (!params->common.threshold_actions.continue_flag)
> > +             /* continue flag not set, break */
> > +             return RESTART_STOP;
> > +
> > +     /* continue action reached, re-enable tracing */
> > +     if (tool->record && trace_instance_start(&tool->record->trace))
> > +             goto err;
> > +     if (tool->aa && trace_instance_start(&tool->aa->trace))
> > +             goto err;
> > +     return RESTART_OK;
> > +
> > +err:
> > +     err_msg("Error restarting trace\n");
> > +     return RESTART_ERROR;
> > +}
>
> The non-BPF functions in common.c have the same logic and should also
> call this.  This isn't timerlat-specific.
>

I will replace them here, thanks.

>
> > diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
> > index 09a3da3f58630..f14fc56c5b4a5 100644
> > --- a/tools/tracing/rtla/src/timerlat_hist.c
> > +++ b/tools/tracing/rtla/src/timerlat_hist.c
> > @@ -1165,18 +1165,19 @@ static int timerlat_hist_bpf_main_loop(struct osnoise_tool *tool)
> >
> >               if (!stop_tracing) {
> >                       /* Threshold overflow, perform actions on threshold */
> > -                     actions_perform(&params->common.threshold_actions);
> > +                     enum restart_result result;
> >
> > -                     if (!params->common.threshold_actions.continue_flag)
> > -                             /* continue flag not set, break */
> > +                     result = timerlat_restart(tool, params);
> > +                     if (result == RESTART_STOP)
> >                               break;
> >
> > -                     /* continue action reached, re-enable tracing */
> > -                     if (tool->record)
> > -                             trace_instance_start(&tool->record->trace);
> > -                     if (tool->aa)
> > -                             trace_instance_start(&tool->aa->trace);
> > -                     timerlat_bpf_restart_tracing();
> > +                     if (result == RESTART_ERROR)
> > +                             return -1;
>
> Does it matter that we're not detaching on an error here?  Is this
> something that gets cleaned up automatically (and if so, why do we ever
> need to do it explicitly)?
>

On process exit, it does.

> > +
> > +                     if (timerlat_bpf_restart_tracing()) {
> > +                             err_msg("Error restarting BPF trace\n");
> > +                             return -1;
> > +                     }
>
> [insert rant about not being able to use exceptions in userspace code in
> the year 2025]
>

I actually find exceptions an anti-pattern. Modern languages like Zig,
Go and Rust came back to error returning.

> -Crystal
>

Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper

[Crystal Wood]

On Tue, 2025-11-25 at 11:20 -0300, Wander Lairson Costa wrote:
> On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
> > 
> > On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
> 
> > > +
> > > +                     if (timerlat_bpf_restart_tracing()) {
> > > +                             err_msg("Error restarting BPF trace\n");
> > > +                             return -1;
> > > +                     }
> > 
> > [insert rant about not being able to use exceptions in userspace code in
> > the year 2025]
> > 
> 
> I actually find exceptions an anti-pattern. Modern languages like Zig,
> Go and Rust came back to error returning.

Maybe I'm behind the times, but I see exceptions and error returns as
complementary... not everything should be an exception and I can
certainly see how they could be overused in an anti-pattern way, but
they're nice for getting useful information out rather than "something
failed" without having to add a bunch of debug prints.

-Crystal

Re: [rtla 07/13] rtla: Introduce timerlat_restart() helper

[Wander Lairson Costa]

On Tue, Nov 25, 2025 at 2:36 PM Crystal Wood <crwood@redhat.com> wrote:
>
> On Tue, 2025-11-25 at 11:20 -0300, Wander Lairson Costa wrote:
> > On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
> > >
> > > On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
> >
> > > > +
> > > > +                     if (timerlat_bpf_restart_tracing()) {
> > > > +                             err_msg("Error restarting BPF trace\n");
> > > > +                             return -1;
> > > > +                     }
> > >
> > > [insert rant about not being able to use exceptions in userspace code in
> > > the year 2025]
> > >
> >
> > I actually find exceptions an anti-pattern. Modern languages like Zig,
> > Go and Rust came back to error returning.
>
> Maybe I'm behind the times, but I see exceptions and error returns as
> complementary... not everything should be an exception and I can
> certainly see how they could be overused in an anti-pattern way, but
> they're nice for getting useful information out rather than "something
> failed" without having to add a bunch of debug prints.
>

IIRC, you do get stack trace from the error returns. I think Golang
didn't/don't by
default. Exception as an alias for "panic()" is fine, IMO. What I've
seen in production
is services down due to unhandled exceptions. One of the reasons is
that there is nothing
in the function signature that says if or what of exceptions it raises.

> -Crystal
>

[rtla 08/13] rtla: Use standard exit codes for result enum

[Wander Lairson Costa]

The result enum defines custom values for PASSED, ERROR, and FAILED.
These values correspond to standard exit codes EXIT_SUCCESS and
EXIT_FAILURE.

Update the enum to use the standard macros EXIT_SUCCESS and
EXIT_FAILURE to improve readability and adherence to standard C
practices.

The FAILED value is implicitly assigned EXIT_FAILURE + 1, so there
is no need to assign an explicit value.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/utils.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index f7ff548f7fba7..beff211fdae2d 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -4,6 +4,7 @@
 #include <time.h>
 #include <sched.h>
 #include <stdbool.h>
+#include <stdlib.h>
 
 /*
  * '18446744073709551615\0'
@@ -97,7 +98,7 @@ bool strtoi(const char *s, int *res);
 #define ns_to_per(total, part) ((part * 100) / (double)total)
 
 enum result {
-	PASSED = 0, /* same as EXIT_SUCCESS */
-	ERROR = 1,  /* same as EXIT_FAILURE, an error in arguments */
-	FAILED = 2, /* test hit the stop tracing condition */
+	PASSED	= EXIT_SUCCESS,
+	ERROR	= EXIT_FAILURE,
+	FAILED, /* test hit the stop tracing condition */
 };
-- 
2.51.1

Re: [rtla 08/13] rtla: Use standard exit codes for result enum

[Wander Lairson Costa]

On Fri, Nov 28, 2025 at 10:18 AM Costa Shulyupin <costa.shul@redhat.com> wrote:
>
> On Mon, 17 Nov 2025 at 20:56, Wander Lairson Costa <wander@redhat.com> wrote:
> >
> >
> > -       PASSED = 0, /* same as EXIT_SUCCESS */
> > -       ERROR = 1,  /* same as EXIT_FAILURE, an error in arguments */
> > -       FAILED = 2, /* test hit the stop tracing condition */
> > +       PASSED  = EXIT_SUCCESS,
> > +       ERROR   = EXIT_FAILURE,
> > +       FAILED, /* test hit the stop tracing condition */
>
> The exit codes are defined as numbers internationally because it provides direct translation from numbers to codes and vice versa. Additional indirection doesn't add to readability.
>

I believe that where it is written "internationally" reads
"intentionally" (happens to me all the time). The main motivation is
to use the standard C library exit codes when calling exit().

> Costa
>
>
> ________________________________
> Hi Wander,
>
> Additional indirection does not add to readability.
>
> Thanks,
> Costa

[rtla 09/13] rtla: Exit if trace output action fails

[Wander Lairson Costa]

The actions_add_trace_output() function can fail if it's unable to
allocate memory for a new action. Currently, the return value is not
checked, and the program continues to run, which can lead to
unexpected behavior.

Add a check to the return value of actions_add_trace_output() and
exit with a proper error message if it fails.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/timerlat_hist.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index f14fc56c5b4a5..39a14c4e07de8 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -1070,8 +1070,10 @@ static struct common_params
 		}
 	}
 
-	if (trace_output)
-		actions_add_trace_output(&params->common.threshold_actions, trace_output);
+	if (trace_output && actions_add_trace_output(&params->common.threshold_actions, trace_output)) {
+		err_msg("Could not add a new trace output");
+		exit(EXIT_FAILURE);
+	}
 
 	if (geteuid()) {
 		err_msg("rtla needs root permission\n");
-- 
2.51.1

[rtla 10/13] rtla: Remove redundant memset after calloc

[Wander Lairson Costa]

The actions struct is allocated using calloc, which already returns
zeroed memory. The subsequent memset call to zero the 'present' member
is therefore redundant.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 2a01ece78454c..2d153d5efdea2 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -21,8 +21,6 @@ actions_init(struct actions *self)
 	self->len = 0;
 	self->continue_flag = false;
 
-	memset(&self->present, 0, sizeof(self->present));
-
 	/* This has to be set by the user */
 	self->trace_output_inst = NULL;
 	return 0;
-- 
2.51.1

[rtla 11/13] rtla: Replace magic number with MAX_PATH

[Wander Lairson Costa]

The trace functions use a buffer to manipulate strings that will be
written to tracefs files. These buffers are defined with a magic number
of 1024, which is a common source of vulnerabilities.

Replace the magic number 1024 with the MAX_PATH macro to make the code
safer and more readable. While at it, replace other instances of the
magic number with ARRAY_SIZE() when the buffer is locally defined.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/osnoise.c    |  4 ++--
 tools/tracing/rtla/src/timerlat_u.c |  4 ++--
 tools/tracing/rtla/src/trace.c      | 20 ++++++++++----------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise.c b/tools/tracing/rtla/src/osnoise.c
index 5075e0f485c77..d502cbbcea91b 100644
--- a/tools/tracing/rtla/src/osnoise.c
+++ b/tools/tracing/rtla/src/osnoise.c
@@ -52,7 +52,7 @@ char *osnoise_get_cpus(struct osnoise_context *context)
 int osnoise_set_cpus(struct osnoise_context *context, char *cpus)
 {
 	char *orig_cpus = osnoise_get_cpus(context);
-	char buffer[1024];
+	char buffer[MAX_PATH];
 	int retval;
 
 	if (!orig_cpus)
@@ -62,7 +62,7 @@ int osnoise_set_cpus(struct osnoise_context *context, char *cpus)
 	if (!context->curr_cpus)
 		return -1;
 
-	snprintf(buffer, 1024, "%s\n", cpus);
+	snprintf(buffer, ARRAY_SIZE(buffer), "%s\n", cpus);
 
 	debug_msg("setting cpus to %s from %s", cpus, context->orig_cpus);
 
diff --git a/tools/tracing/rtla/src/timerlat_u.c b/tools/tracing/rtla/src/timerlat_u.c
index 01dbf9a6b5a51..52977e725c79c 100644
--- a/tools/tracing/rtla/src/timerlat_u.c
+++ b/tools/tracing/rtla/src/timerlat_u.c
@@ -32,7 +32,7 @@
 static int timerlat_u_main(int cpu, struct timerlat_u_params *params)
 {
 	struct sched_param sp = { .sched_priority = 95 };
-	char buffer[1024];
+	char buffer[MAX_PATH];
 	int timerlat_fd;
 	cpu_set_t set;
 	int retval;
@@ -87,7 +87,7 @@ static int timerlat_u_main(int cpu, struct timerlat_u_params *params)
 
 	/* add should continue with a signal handler */
 	while (true) {
-		retval = read(timerlat_fd, buffer, 1024);
+		retval = read(timerlat_fd, buffer, ARRAY_SIZE(buffer));
 		if (retval < 0)
 			break;
 	}
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 813f4368f104b..658a6e94edfba 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -330,7 +330,7 @@ int trace_event_add_trigger(struct trace_events *event, char *trigger)
 static void trace_event_disable_filter(struct trace_instance *instance,
 				       struct trace_events *tevent)
 {
-	char filter[1024];
+	char filter[MAX_PATH];
 	int retval;
 
 	if (!tevent->filter)
@@ -342,7 +342,7 @@ static void trace_event_disable_filter(struct trace_instance *instance,
 	debug_msg("Disabling %s:%s filter %s\n", tevent->system,
 		  tevent->event ? : "*", tevent->filter);
 
-	snprintf(filter, 1024, "!%s\n", tevent->filter);
+	snprintf(filter, ARRAY_SIZE(filter), "!%s\n", tevent->filter);
 
 	retval = tracefs_event_file_write(instance->inst, tevent->system,
 					  tevent->event, "filter", filter);
@@ -361,7 +361,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
 {
 	int retval, index, out_fd;
 	mode_t mode = 0644;
-	char path[1024];
+	char path[MAX_PATH];
 	char *hist;
 
 	if (!tevent)
@@ -376,7 +376,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
 	if (retval)
 		return;
 
-	snprintf(path, 1024, "%s_%s_hist.txt", tevent->system, tevent->event);
+	snprintf(path, ARRAY_SIZE(path), "%s_%s_hist.txt", tevent->system, tevent->event);
 
 	printf("  Saving event %s:%s hist to %s\n", tevent->system, tevent->event, path);
 
@@ -408,7 +408,7 @@ static void trace_event_save_hist(struct trace_instance *instance,
 static void trace_event_disable_trigger(struct trace_instance *instance,
 					struct trace_events *tevent)
 {
-	char trigger[1024];
+	char trigger[MAX_PATH];
 	int retval;
 
 	if (!tevent->trigger)
@@ -422,7 +422,7 @@ static void trace_event_disable_trigger(struct trace_instance *instance,
 
 	trace_event_save_hist(instance, tevent);
 
-	snprintf(trigger, 1024, "!%s\n", tevent->trigger);
+	snprintf(trigger, ARRAY_SIZE(trigger), "!%s\n", tevent->trigger);
 
 	retval = tracefs_event_file_write(instance->inst, tevent->system,
 					  tevent->event, "trigger", trigger);
@@ -461,7 +461,7 @@ void trace_events_disable(struct trace_instance *instance,
 static int trace_event_enable_filter(struct trace_instance *instance,
 				     struct trace_events *tevent)
 {
-	char filter[1024];
+	char filter[MAX_PATH];
 	int retval;
 
 	if (!tevent->filter)
@@ -473,7 +473,7 @@ static int trace_event_enable_filter(struct trace_instance *instance,
 		return 1;
 	}
 
-	snprintf(filter, 1024, "%s\n", tevent->filter);
+	snprintf(filter, ARRAY_SIZE(filter), "%s\n", tevent->filter);
 
 	debug_msg("Enabling %s:%s filter %s\n", tevent->system,
 		  tevent->event ? : "*", tevent->filter);
@@ -496,7 +496,7 @@ static int trace_event_enable_filter(struct trace_instance *instance,
 static int trace_event_enable_trigger(struct trace_instance *instance,
 				      struct trace_events *tevent)
 {
-	char trigger[1024];
+	char trigger[MAX_PATH];
 	int retval;
 
 	if (!tevent->trigger)
@@ -508,7 +508,7 @@ static int trace_event_enable_trigger(struct trace_instance *instance,
 		return 1;
 	}
 
-	snprintf(trigger, 1024, "%s\n", tevent->trigger);
+	snprintf(trigger, ARRAY_SIZE(trigger), "%s\n", tevent->trigger);
 
 	debug_msg("Enabling %s:%s trigger %s\n", tevent->system,
 		  tevent->event ? : "*", tevent->trigger);
-- 
2.51.1

[rtla 12/13] rtla: Remove unused headers

[Wander Lairson Costa]

Remove unused includes for <errno.h> and <signal.h> to clean up the
code and reduce unnecessary dependencies.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/osnoise_hist.c | 1 -
 tools/tracing/rtla/src/timerlat.c     | 1 -
 tools/tracing/rtla/src/timerlat_top.c | 1 -
 tools/tracing/rtla/src/trace.c        | 1 -
 4 files changed, 4 deletions(-)

diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c
index dffb6d0a98d7d..44614f56493f2 100644
--- a/tools/tracing/rtla/src/osnoise_hist.c
+++ b/tools/tracing/rtla/src/osnoise_hist.c
@@ -9,7 +9,6 @@
 #include <string.h>
 #include <signal.h>
 #include <unistd.h>
-#include <errno.h>
 #include <stdio.h>
 #include <time.h>
 
diff --git a/tools/tracing/rtla/src/timerlat.c b/tools/tracing/rtla/src/timerlat.c
index 50c7eb00fd6b7..9b77dbcbd1375 100644
--- a/tools/tracing/rtla/src/timerlat.c
+++ b/tools/tracing/rtla/src/timerlat.c
@@ -9,7 +9,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
 #include <sched.h>
diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c
index d831a9e1818f4..7f2491e72e495 100644
--- a/tools/tracing/rtla/src/timerlat_top.c
+++ b/tools/tracing/rtla/src/timerlat_top.c
@@ -11,7 +11,6 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <time.h>
-#include <errno.h>
 #include <sched.h>
 #include <pthread.h>
 
diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
index 658a6e94edfba..ab2bf6d04f2ee 100644
--- a/tools/tracing/rtla/src/trace.c
+++ b/tools/tracing/rtla/src/trace.c
@@ -2,7 +2,6 @@
 #define _GNU_SOURCE
 #include <sys/sendfile.h>
 #include <tracefs.h>
-#include <signal.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <errno.h>
-- 
2.51.1

[rtla 13/13] rtla: Fix inconsistent state in actions_add_* functions

[Wander Lairson Costa]

The actions_add_trace_output() and actions_add_shell() functions
leave the action list in an inconsistent state when strdup() fails.
The actions_new() function increments self->len before returning a
pointer to the new action slot, but if the subsequent strdup()
allocation fails, the function returns an error without decrementing
self->len back.

This leaves an action object in an invalid state within the list.
When actions_destroy() or other functions iterate over the list
using for_each_action(), they will access this invalid entry with
uninitialized fields, potentially leading to undefined behavior.

Fix this by decrementing self->len when strdup() fails, effectively
returning the allocated slot back to the pool and maintaining list
consistency even when memory allocation fails.

Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 tools/tracing/rtla/src/actions.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index 2d153d5efdea2..4aaaedadcc42a 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -76,11 +76,13 @@ actions_add_trace_output(struct actions *self, const char *trace_output)
 	if (!action)
 		return -1;
 
-	self->present[ACTION_TRACE_OUTPUT] = true;
 	action->type = ACTION_TRACE_OUTPUT;
 	action->trace_output = strdup(trace_output);
-	if (!action->trace_output)
+	if (!action->trace_output) {
+		self->len--; // return the action object to the pool
 		return -1;
+	}
+	self->present[ACTION_TRACE_OUTPUT] = true;
 
 	return 0;
 }
@@ -115,11 +117,13 @@ actions_add_shell(struct actions *self, const char *command)
 	if (!action)
 		return -1;
 
-	self->present[ACTION_SHELL] = true;
 	action->type = ACTION_SHELL;
 	action->command = strdup(command);
-	if (!action->command)
+	if (!action->command) {
+		self->len--;
 		return -1;
+	}
+	self->present[ACTION_SHELL] = true;
 
 	return 0;
 }
-- 
2.51.1