[PATCH v2 0/6] rtla: Migrate to libsubcmd for command line option parsing

Tomas Glozar posted 6 patches 3 days, 5 hours ago
Documentation/tools/rtla/common_appendix.txt  |   7 +-
tools/lib/subcmd/parse-options.c              |  63 +-
tools/lib/subcmd/parse-options.h              |   4 +
tools/tracing/rtla/.gitignore                 |   2 +
tools/tracing/rtla/Makefile                   |  66 +-
tools/tracing/rtla/src/Build                  |   2 +-
tools/tracing/rtla/src/cli.c                  | 537 +++++++++++++
tools/tracing/rtla/src/cli.h                  |   9 +
tools/tracing/rtla/src/cli_p.h                | 670 +++++++++++++++++
tools/tracing/rtla/src/common.c               | 109 ---
tools/tracing/rtla/src/common.h               |  27 +-
tools/tracing/rtla/src/osnoise.c              |   9 +-
tools/tracing/rtla/src/osnoise_hist.c         | 221 +-----
tools/tracing/rtla/src/osnoise_top.c          | 200 +----
tools/tracing/rtla/src/rtla.c                 |  92 ---
tools/tracing/rtla/src/timerlat.c             |   9 +-
tools/tracing/rtla/src/timerlat.h             |   6 +-
tools/tracing/rtla/src/timerlat_hist.c        | 317 +-------
tools/tracing/rtla/src/timerlat_top.c         | 286 +------
tools/tracing/rtla/src/utils.c                |  28 +-
tools/tracing/rtla/src/utils.h                |   9 +-
tools/tracing/rtla/tests/hwnoise.t            |   2 +-
tools/tracing/rtla/tests/osnoise.t            |   6 +-
tools/tracing/rtla/tests/timerlat.t           |   6 +-
tools/tracing/rtla/tests/unit/Build           |   5 +
tools/tracing/rtla/tests/unit/Makefile.unit   |   4 +-
.../rtla/tests/unit/cli_opt_callback.c        | 704 ++++++++++++++++++
.../rtla/tests/unit/cli_params_assert.h       |  68 ++
.../rtla/tests/unit/osnoise_hist_cli.c        | 557 ++++++++++++++
.../tracing/rtla/tests/unit/osnoise_top_cli.c | 503 +++++++++++++
.../rtla/tests/unit/timerlat_hist_cli.c       | 702 +++++++++++++++++
.../rtla/tests/unit/timerlat_top_cli.c        | 634 ++++++++++++++++
tools/tracing/rtla/tests/unit/unit_tests.c    |  13 +
33 files changed, 4566 insertions(+), 1311 deletions(-)
create mode 100644 tools/tracing/rtla/src/cli.c
create mode 100644 tools/tracing/rtla/src/cli.h
create mode 100644 tools/tracing/rtla/src/cli_p.h
delete mode 100644 tools/tracing/rtla/src/rtla.c
create mode 100644 tools/tracing/rtla/tests/unit/cli_opt_callback.c
create mode 100644 tools/tracing/rtla/tests/unit/cli_params_assert.h
create mode 100644 tools/tracing/rtla/tests/unit/osnoise_hist_cli.c
create mode 100644 tools/tracing/rtla/tests/unit/osnoise_top_cli.c
create mode 100644 tools/tracing/rtla/tests/unit/timerlat_hist_cli.c
create mode 100644 tools/tracing/rtla/tests/unit/timerlat_top_cli.c
[PATCH v2 0/6] rtla: Migrate to libsubcmd for command line option parsing
Posted by Tomas Glozar 3 days, 5 hours ago
[ CC to linux-perf-users for the libsubcmd code changes ]

rtla currently uses its own implementation that uses getopt_long() to
parse command-line arguments.

Migrate rtla to use libsubcmd for command line argument parsing,
similarly to what is already done by other tools like perf, bpftool,
and objtool. Among other benefits, this allows help messages to be
generated automatically rather than having to be typed out manually
for each tool.

libsubcmd is extended with a flag to parse optarg from separate
argument if a new flag is turned on. Without the flag, the old behavior
is preserved. That keeps the parsing working for tools that use
positional arguments, and allows RTLA to keep its flexible syntax for -C
and -t options and their long variants, --cgroup and --trace-output.
Another flag is added to disable automatic definition of --no-xy for
every option --xy and vice versa, which overlaps for RTLA's --irq and
--thread options.

The new implementation is moved into a separate file, cli.c, together
with a tiny header counterpart, cli.h. This helps separate the parsing
logic, which has little in common with the rest of RTLA, in a separate
module. Another new file, cli_p.h, is used as a private header to contain
macros and static function declarations that are also used by unit tests
next to cli.c, but should not be imported from elsewhere.

Macros to generate struct option array fields for libsubcmd's
parse_args() are used to preserve the consolidation of argument parsing
code across different RTLA tools. Kernel and user threads are, as
an exception, treated as common, although they are currently implemented
for timerlat only, in line with earlier consolidation changes.

The test suite is expanded to include two levels of unit tests, one testing
the already existing tool_parse_args() functions, one tests option callbacks,
which are a new level of the CLI parser added in this patchset. This helps
to verify that no regressions are caused by this refactoring.

I expect more improvements to the code being possible in the future,
like creating macros for option groups to further deduplicate the code,
reducing the amount of extra code in the _parse_args() functions, or
implementing support for unsetting options (which is currently only
supported for those that do not use a custom callback).

v2 changes:
- Two unit test suites are added to cover regressions, after several
options were broken by the v1. The test suites cover all parsing
issues reported in the v1 as well as those found during additional
testing.
- Return value of all paths that print help, including those that
are handled in RTLA, are set to libsubcmd's help exit code of 129.
Previously, only the tool help returned 129. While some other tools
(e.g. bpftool) do that, RTLA unifies those for consistency. The return
value is also added to the corresponding section in documentation.
- Incorrect parsing of --no-irq and --no-thread is fixed using a newly
added libsubcmd option flag.
- Incorrect parsing of -n and -u timerlat options (which erroneously
required an argument in v1) is fixed.
- A now stale declaration of removed function common_parse_options()
is removed from common.h.
- Segmentation fault on abbreviated --help (e.g. --he) is fixed.
- Incorrect formatting of OPT_END macro (spurious tab) is fixed.
- All opt_* callbacks now reject unimplemented unset (--no-) correctly.
- opt_trigger_cb() and opt_filter_cb() now take only the required events
field, not the whole params structure.
- Spurious opt_osnoise_threshold_cb() which is actually just a wrapper for
opt_llong_callback() is removed.
- Old off-by-one typo is fixed in --dma-latency and -E/--entries error
messages, to make it consistent with the newly added unit tests.
- Fix a bug in Makefile that defined LIBSUBCMD_INCLUDES as -I... and then
used it as a target name: define it as the path only, and then add -I$(...)
to CFLAGS, as this is the only include path that is generated during the build
itself.

v1: https://lore.kernel.org/linux-trace-kernel/20260320150651.51057-1-tglozar@redhat.com/T

Tomas Glozar (6):
  rtla: Add libsubcmd dependency
  tools subcmd: support optarg as separate argument
  tools subcmd: allow parsing distinct --opt and --no-opt
  rtla: Parse cmdline using libsubcmd
  rtla/tests: Add unit tests for _parse_args() functions
  rtla/tests: Add unit tests for CLI option callbacks

 Documentation/tools/rtla/common_appendix.txt  |   7 +-
 tools/lib/subcmd/parse-options.c              |  63 +-
 tools/lib/subcmd/parse-options.h              |   4 +
 tools/tracing/rtla/.gitignore                 |   2 +
 tools/tracing/rtla/Makefile                   |  66 +-
 tools/tracing/rtla/src/Build                  |   2 +-
 tools/tracing/rtla/src/cli.c                  | 537 +++++++++++++
 tools/tracing/rtla/src/cli.h                  |   9 +
 tools/tracing/rtla/src/cli_p.h                | 670 +++++++++++++++++
 tools/tracing/rtla/src/common.c               | 109 ---
 tools/tracing/rtla/src/common.h               |  27 +-
 tools/tracing/rtla/src/osnoise.c              |   9 +-
 tools/tracing/rtla/src/osnoise_hist.c         | 221 +-----
 tools/tracing/rtla/src/osnoise_top.c          | 200 +----
 tools/tracing/rtla/src/rtla.c                 |  92 ---
 tools/tracing/rtla/src/timerlat.c             |   9 +-
 tools/tracing/rtla/src/timerlat.h             |   6 +-
 tools/tracing/rtla/src/timerlat_hist.c        | 317 +-------
 tools/tracing/rtla/src/timerlat_top.c         | 286 +------
 tools/tracing/rtla/src/utils.c                |  28 +-
 tools/tracing/rtla/src/utils.h                |   9 +-
 tools/tracing/rtla/tests/hwnoise.t            |   2 +-
 tools/tracing/rtla/tests/osnoise.t            |   6 +-
 tools/tracing/rtla/tests/timerlat.t           |   6 +-
 tools/tracing/rtla/tests/unit/Build           |   5 +
 tools/tracing/rtla/tests/unit/Makefile.unit   |   4 +-
 .../rtla/tests/unit/cli_opt_callback.c        | 704 ++++++++++++++++++
 .../rtla/tests/unit/cli_params_assert.h       |  68 ++
 .../rtla/tests/unit/osnoise_hist_cli.c        | 557 ++++++++++++++
 .../tracing/rtla/tests/unit/osnoise_top_cli.c | 503 +++++++++++++
 .../rtla/tests/unit/timerlat_hist_cli.c       | 702 +++++++++++++++++
 .../rtla/tests/unit/timerlat_top_cli.c        | 634 ++++++++++++++++
 tools/tracing/rtla/tests/unit/unit_tests.c    |  13 +
 33 files changed, 4566 insertions(+), 1311 deletions(-)
 create mode 100644 tools/tracing/rtla/src/cli.c
 create mode 100644 tools/tracing/rtla/src/cli.h
 create mode 100644 tools/tracing/rtla/src/cli_p.h
 delete mode 100644 tools/tracing/rtla/src/rtla.c
 create mode 100644 tools/tracing/rtla/tests/unit/cli_opt_callback.c
 create mode 100644 tools/tracing/rtla/tests/unit/cli_params_assert.h
 create mode 100644 tools/tracing/rtla/tests/unit/osnoise_hist_cli.c
 create mode 100644 tools/tracing/rtla/tests/unit/osnoise_top_cli.c
 create mode 100644 tools/tracing/rtla/tests/unit/timerlat_hist_cli.c
 create mode 100644 tools/tracing/rtla/tests/unit/timerlat_top_cli.c

-- 
2.54.0