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

[Wander Lairson Costa <wander@redhat.com>]

On Thu, May 21, 2026 at 04:18:27PM +0200, Tomas Glozar wrote:
> [ 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

Reviewed-by: Wander Lairson Costa <wander@redhat.com>