rtla: Fix parsing of multi-character short options
A bug was reported where the parsing of multi-character short options,
be it a short option with an argument specified without space (e.g.
"-p100") or multiple short options in one argument (e.g. -un), ignores
options specific to individual tools.
Furthermore, if the rest of the option is supposed to be an argument, it
gets reinterpreted as a string of options. For example, -p100 gets
interpreted as -100, which is due to hackish implementation read as
--no-thread --no-irq --no-irq with timerlat hist, causing rtla to error
out:
$ rtla timerlat hist -p100
no-irq and no-thread set, there is nothing to do here
This behavior is caused by getopt_long() being called twice on each
argument, once in common_parse_options(), once in [tool]_parse_args():
- common_parse_options() calls getopt_long() with an array of options
common for all rtla tools, while suppressing errors (opterr = 0).
- If the option fails to parse, common_parse_options() returns 0.
- If 0 is returned from common_parse_options(), [tool]_parse_args()
calls getopt_long() again, with its own set of options.
* [tool] means one of {osnoise,timerlat}_{top,hist}
At least in glibc, getopt_long() increments its internal nextchar
variable even if the option is not recognized. That means that in the
case of "-p100", common_parse_options() sets nextchar pointing to '1',
and timerlat_hist_parse_args() sees '1', not 'p'; the same then repeats
for the first and second '0'.
As there is no way to restore the correct internal state of
getopt_long() reliably, fix the issue by merging the common options back
to the longopt array and option string of the [tool]_parse_args()
functions using a macro; only the switch part is left in the original
function, which is renamed to set_common_option().
Fixes: 850cd24cb6 ("tools/rtla: Add common_parse_options()")
Reported-by: John Kacur <jkacur@redhat.com>
Tested-by: John Kacur <jkacur@redhat.com>
Link: https://lore.kernel.org/r/20260602125506.3325345-1-tglozar@redhat.com
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
This commit is contained in:
parent
e43ffb69e0
commit
e9e41d3035
6 changed files with 32 additions and 36 deletions
|
|
@ -84,37 +84,20 @@ int getopt_auto(int argc, char **argv, const struct option *long_opts)
|
|||
}
|
||||
|
||||
/*
|
||||
* common_parse_options - parse common command line options
|
||||
* set_common_option - set common options
|
||||
*
|
||||
* @c: option character
|
||||
* @argc: argument count
|
||||
* @argv: argument vector
|
||||
* @common: common parameters structure
|
||||
*
|
||||
* Parse command line options that are common to all rtla tools.
|
||||
*
|
||||
* Returns: non zero if a common option was parsed, or 0
|
||||
* if the option should be handled by tool-specific parsing.
|
||||
* Returns: 1 if the option was set, 0 otherwise.
|
||||
*/
|
||||
int common_parse_options(int argc, char **argv, struct common_params *common)
|
||||
int set_common_option(int c, int argc, char **argv, struct common_params *common)
|
||||
{
|
||||
struct trace_events *tevent;
|
||||
int saved_state = optind;
|
||||
int c;
|
||||
|
||||
static struct option long_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'},
|
||||
{"house-keeping", required_argument, 0, 'H'},
|
||||
{"priority", required_argument, 0, 'P'},
|
||||
{0, 0, 0, 0}
|
||||
};
|
||||
|
||||
opterr = 0;
|
||||
c = getopt_auto(argc, argv, long_options);
|
||||
opterr = 1;
|
||||
|
||||
switch (c) {
|
||||
case 'c':
|
||||
|
|
@ -154,11 +137,10 @@ int common_parse_options(int argc, char **argv, struct common_params *common)
|
|||
common->set_sched = 1;
|
||||
break;
|
||||
default:
|
||||
optind = saved_state;
|
||||
return 0;
|
||||
}
|
||||
|
||||
return c;
|
||||
return 1;
|
||||
}
|
||||
|
||||
/*
|
||||
|
|
|
|||
|
|
@ -178,7 +178,17 @@ int osnoise_set_stop_total_us(struct osnoise_context *context,
|
|||
long long stop_total_us);
|
||||
|
||||
int getopt_auto(int argc, char **argv, const struct option *long_opts);
|
||||
int common_parse_options(int argc, char **argv, struct common_params *common);
|
||||
|
||||
#define 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'},\
|
||||
{"house-keeping", required_argument, 0, 'H'},\
|
||||
{"priority", required_argument, 0, 'P'}
|
||||
int set_common_option(int c, int argc, char **argv, struct common_params *common);
|
||||
|
||||
int common_apply_config(struct osnoise_tool *tool, struct common_params *params);
|
||||
int top_main_loop(struct osnoise_tool *tool);
|
||||
int hist_main_loop(struct osnoise_tool *tool);
|
||||
|
|
|
|||
|
|
@ -475,6 +475,7 @@ static struct common_params
|
|||
|
||||
while (1) {
|
||||
static struct option long_options[] = {
|
||||
COMMON_OPTIONS,
|
||||
{"auto", required_argument, 0, 'a'},
|
||||
{"bucket-size", required_argument, 0, 'b'},
|
||||
{"entries", required_argument, 0, 'E'},
|
||||
|
|
@ -498,15 +499,15 @@ static struct common_params
|
|||
{0, 0, 0, 0}
|
||||
};
|
||||
|
||||
if (common_parse_options(argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
c = getopt_auto(argc, argv, long_options);
|
||||
|
||||
/* detect the end of the options. */
|
||||
if (c == -1)
|
||||
break;
|
||||
|
||||
if (set_common_option(c, argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
switch (c) {
|
||||
case 'a':
|
||||
/* set sample stop to auto_thresh */
|
||||
|
|
|
|||
|
|
@ -328,6 +328,7 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
|
|||
|
||||
while (1) {
|
||||
static struct option long_options[] = {
|
||||
COMMON_OPTIONS,
|
||||
{"auto", required_argument, 0, 'a'},
|
||||
{"help", no_argument, 0, 'h'},
|
||||
{"period", required_argument, 0, 'p'},
|
||||
|
|
@ -346,15 +347,15 @@ struct common_params *osnoise_top_parse_args(int argc, char **argv)
|
|||
{0, 0, 0, 0}
|
||||
};
|
||||
|
||||
if (common_parse_options(argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
c = getopt_auto(argc, argv, long_options);
|
||||
|
||||
/* Detect the end of the options. */
|
||||
if (c == -1)
|
||||
break;
|
||||
|
||||
if (set_common_option(c, argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
switch (c) {
|
||||
case 'a':
|
||||
/* set sample stop to auto_thresh */
|
||||
|
|
|
|||
|
|
@ -785,6 +785,7 @@ static struct common_params
|
|||
|
||||
while (1) {
|
||||
static struct option long_options[] = {
|
||||
COMMON_OPTIONS,
|
||||
{"auto", required_argument, 0, 'a'},
|
||||
{"bucket-size", required_argument, 0, 'b'},
|
||||
{"entries", required_argument, 0, 'E'},
|
||||
|
|
@ -819,11 +820,11 @@ static struct common_params
|
|||
{0, 0, 0, 0}
|
||||
};
|
||||
|
||||
if (common_parse_options(argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
c = getopt_auto(argc, argv, long_options);
|
||||
|
||||
if (set_common_option(c, argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
/* detect the end of the options. */
|
||||
if (c == -1)
|
||||
break;
|
||||
|
|
|
|||
|
|
@ -549,6 +549,7 @@ static struct common_params
|
|||
|
||||
while (1) {
|
||||
static struct option long_options[] = {
|
||||
COMMON_OPTIONS,
|
||||
{"auto", required_argument, 0, 'a'},
|
||||
{"help", no_argument, 0, 'h'},
|
||||
{"irq", required_argument, 0, 'i'},
|
||||
|
|
@ -577,11 +578,11 @@ static struct common_params
|
|||
{0, 0, 0, 0}
|
||||
};
|
||||
|
||||
if (common_parse_options(argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
c = getopt_auto(argc, argv, long_options);
|
||||
|
||||
if (set_common_option(c, argc, argv, ¶ms->common))
|
||||
continue;
|
||||
|
||||
/* detect the end of the options. */
|
||||
if (c == -1)
|
||||
break;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue