čt 15. 1. 2026 v 18:25 odesílatel Wander Lairson Costa
<[email protected]> 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 <[email protected]>
> ---
> 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/[email protected]/
> +#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
