Re: [PATCH v3 03/18] rtla: Simplify argument parsing

[Tomas Glozar]

čt 15. 1. 2026 v 18:25 odesílatel Wander Lairson Costa
<wander@xxxxxxxxxx> napsal:
>
> 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@xxxxxxxxxx>
> ---
>  tools/tracing/rtla/src/actions.c | 57 +++++++++++++++++++++++---------
>  tools/tracing/rtla/src/utils.h   | 14 ++++++--
>  2 files changed, 54 insertions(+), 17 deletions(-)
>

Suggestion in case you will be sending a v4 of the patchset: the
commit description could be more descriptive, something like,
"rtla/actions: Simplify argument parsing", or "rtla: Simply action
argument parsing", to make it clear this affects *action* arguments,
not command-line argument parsing in general.

> diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> index e29c2eb5d569d..8323c999260c2 100644
> --- a/tools/tracing/rtla/src/utils.h
> +++ b/tools/tracing/rtla/src/utils.h
> @@ -14,8 +14,18 @@
>  #define MAX_NICE               20
>  #define MIN_NICE               -19
>
> -#define container_of(ptr, type, member)({                      \
> -       const typeof(((type *)0)->member) *__mptr = (ptr);      \

As Crystal pointed out already [1], this part removes container_of,
only to restore it (see below).

[1] https://lore.kernel.org/linux-trace-kernel/301c25518ff81dc967187497a0577f8ee2fb2e29.camel@xxxxxxxxxx/

> +#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, ARRAY_SIZE(s2))
> +
> +#define container_of(ptr, type, member)({                              \
> +       const typeof(((type *)0)->member) * __mptr = (ptr);             \
>         (type *)((char *)__mptr - offsetof(type, member)) ; })
>

...here, container_of is restored, only with one more tab which is
unnecessary - that is why Git includes it into the diff. If you remove
one tab and update the commit, the change should be dropped.

Otherwise, looks good!

Tomas