[PATCH 1/2] rtla/timerlat: Fix parsing of short options with attached arguments

[John Kacur <jkacur@redhat.com>]

The timerlat hist command fails to parse short options with attached
numeric arguments (e.g., -p100) due to conflicts between digit characters
used as option values and numeric arguments to other options.

This issue was discovered when testing rtla 7.1.0-rc6 with rteval,
which passes arguments in the compact -p100 format. The rteval tests
failed with the confusing error "no-irq and no-thread set, there is
nothing to do here" even though neither option was specified.

The root cause is two-fold:

1. Digit characters ('0'-'9') were used as short option values for
   long-only options like --no-irq, --no-thread, etc. This caused
   getopt_auto() to generate an option string like 'a:b:...:u0123456:7:8:9'
   which made getopt treat digits as valid option characters.

2. The two-phase option parsing approach (alternating calls between
   common_parse_options() and local option parsing) confused getopt's
   internal state when encountering arguments like -p100.

When a user passed -p100, getopt would incorrectly parse it as three
separate options: -p, -1, -0, and -0, silently setting no_irq and
no_thread flags instead of recognizing "100" as the period argument.

The two-phase parsing was introduced in commit 850cd24cb6d6 ("tools/rtla:
Add common_parse_options()") which first appeared in v7.0-rc1. Prior to
that commit, -p100 worked correctly. The digit characters as option
values existed since the original timerlat implementation, but only
became problematic when combined with the two-phase parsing approach.

Fix this by:

1. Eliminating digit characters from the option string by filtering them
   out in getopt_auto(). This prevents conflicts with numeric arguments.

2. Refactoring timerlat_hist_parse_args() to use single-pass option
   parsing. Instead of alternating between common_parse_options() and
   local parsing, merge all options (common and local) into a single
   option table and parse them in one pass. This matches the approach
   used by cyclictest and other tools.

With these changes, all argument formats work correctly:
  -p 100        (short with space)
  -p100         (short without space)
  --period=100  (long with =)
  --period 100  (long with space)

This maintains compatibility with existing usage while enabling the
compact -p100 format that users expect from similar tools.

Assisted-by: Claude:claude-sonnet-4-5
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 tools/tracing/rtla/src/common.c        |  4 ++
 tools/tracing/rtla/src/timerlat_hist.c | 55 ++++++++++++++++++++++++--
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index 35e3d3aa922e..c2fd051c562c 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -65,6 +65,10 @@ int getopt_auto(int argc, char **argv, const struct option *long_opts)
 		if (long_opts[i].val < 32 || long_opts[i].val > 127)
 			continue;
 
+		/* Skip digit characters to avoid conflicts with numeric arguments */
+		if (long_opts[i].val >= '0' && long_opts[i].val <= '9')
+			continue;
+
 		if (n + 4 >= sizeof(opts))
 			fatal("optstring buffer overflow");
 
diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c
index 79142af4f566..c0b6d7c30114 100644
--- a/tools/tracing/rtla/src/timerlat_hist.c
+++ b/tools/tracing/rtla/src/timerlat_hist.c
@@ -787,11 +787,24 @@ static struct common_params
 		static struct option long_options[] = {
 			{"auto",		required_argument,	0, 'a'},
 			{"bucket-size",		required_argument,	0, 'b'},
+			/* Common options */
+			{"cpus",		required_argument,	0, 'c'},
+			{"cgroup",		optional_argument,	0, 'C'},
+			{"debug",		no_argument,		0, 'D'},
+			{"duration",		required_argument,	0, 'd'},
+			{"event",		required_argument,	0, 'e'},
+			/* End common options */
 			{"entries",		required_argument,	0, 'E'},
 			{"help",		no_argument,		0, 'h'},
+			/* Common option */
+			{"house-keeping",	required_argument,	0, 'H'},
+			/* End common option */
 			{"irq",			required_argument,	0, 'i'},
 			{"nano",		no_argument,		0, 'n'},
 			{"period",		required_argument,	0, 'p'},
+			/* Common option */
+			{"priority",		required_argument,	0, 'P'},
+			/* End common option */
 			{"stack",		required_argument,	0, 's'},
 			{"thread",		required_argument,	0, 'T'},
 			{"trace",		optional_argument,	0, 't'},
@@ -819,9 +832,6 @@ static struct common_params
 			{0, 0, 0, 0}
 		};
 
-		if (common_parse_options(argc, argv, &params->common))
-			continue;
-
 		c = getopt_auto(argc, argv, long_options);
 
 		/* detect the end of the options. */
@@ -850,6 +860,35 @@ static struct common_params
 			    params->common.hist.bucket_size >= 1000000)
 				fatal("Bucket size needs to be > 0 and <= 1000000");
 			break;
+		case 'c':
+			if (parse_cpu_set(optarg, &params->common.monitored_cpus))
+				fatal("Invalid -c cpu list");
+			params->common.cpus = optarg;
+			break;
+		case 'C':
+			params->common.cgroup = 1;
+			params->common.cgroup_name = parse_optional_arg(argc, argv);
+			break;
+		case 'D':
+			config_debug = 1;
+			break;
+		case 'd':
+			params->common.duration = parse_seconds_duration(optarg);
+			if (!params->common.duration)
+				fatal("Invalid -d duration");
+			break;
+		case 'e':
+			{
+				struct trace_events *tevent;
+				tevent = trace_event_alloc(optarg);
+				if (!tevent)
+					fatal("Error alloc trace event");
+
+				if (params->common.events)
+					tevent->next = params->common.events;
+				params->common.events = tevent;
+			}
+			break;
 		case 'E':
 			params->common.hist.entries = get_llong_from_str(optarg);
 			if (params->common.hist.entries < 10 ||
@@ -860,6 +899,11 @@ static struct common_params
 		case '?':
 			timerlat_hist_usage();
 			break;
+		case 'H':
+			params->common.hk_cpus = 1;
+			if (parse_cpu_set(optarg, &params->common.hk_cpu_set))
+				fatal("Error parsing house keeping CPUs");
+			break;
 		case 'i':
 			params->common.stop_us = get_llong_from_str(optarg);
 			break;
@@ -874,6 +918,11 @@ static struct common_params
 			if (params->timerlat_period_us > 1000000)
 				fatal("Period longer than 1 s");
 			break;
+		case 'P':
+			if (parse_prio(optarg, &params->common.sched_param) == -1)
+				fatal("Invalid -P priority");
+			params->common.set_sched = 1;
+			break;
 		case 's':
 			params->print_stack = get_llong_from_str(optarg);
 			break;
-- 
2.54.0