[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e96f06667d07fe7f207fc8769218967d22cf7634.camel@redhat.com>
Date: Mon, 24 Nov 2025 18:46:33 -0600
From: Crystal Wood <crwood@...hat.com>
To: Wander Lairson Costa <wander@...hat.com>, Steven Rostedt
<rostedt@...dmis.org>, Tomas Glozar <tglozar@...hat.com>, Ivan Pravdin
<ipravdin.official@...il.com>, John Kacur <jkacur@...hat.com>, Costa
Shulyupin <costa.shul@...hat.com>, Tiezhu Yang <yangtiezhu@...ngson.cn>,
"open list:Real-time Linux Analysis (RTLA) tools"
<linux-trace-kernel@...r.kernel.org>, open list
<linux-kernel@...r.kernel.org>, "open list:BPF
[MISC]:Keyword:(?:\\b|_)bpf(?:\\b|_)" <bpf@...r.kernel.org>
Subject: Re: [rtla 05/13] rtla: Simplify argument parsing
On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> +/*
> + * extract_arg - extract argument value from option token
> + * @token: option token (e.g., "file=trace.txt")
> + * @opt: option name to match (e.g., "file")
> + *
> + * Returns pointer to argument value after "=" if token matches "opt=",
> + * otherwise returns NULL.
> + */
> +#define extract_arg(token, opt) ( \
> + strlen(token) > STRING_LENGTH(opt "=") && \
> + !strncmp_static(token, opt "=") \
> + ? (token) + STRING_LENGTH(opt "=") : NULL )
This could be implemented as a function (albeit without the
concatenation trick)... but if it must be a macro, at least encase it
with ({ }) so it behaves more like a function.
> +
> /*
> * actions_parse - add an action based on text specification
> */
> int
> actions_parse(struct actions *self, const char *trigger, const char *tracefn)
> {
> +
> enum action_type type = ACTION_NONE;
Why this blank line?
> diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
> index 160491f5de91c..f7ff548f7fba7 100644
> --- a/tools/tracing/rtla/src/utils.h
> +++ b/tools/tracing/rtla/src/utils.h
> @@ -13,8 +13,18 @@
> #define MAX_NICE 20
> #define MIN_NICE -19
>
> -#define container_of(ptr, type, member)({ \
> - const typeof(((type *)0)->member) *__mptr = (ptr); \
[snip]
> +#define container_of(ptr, type, member)({ \
> + const typeof(((type *)0)->member) *__mptr = (ptr); \
> (type *)((char *)__mptr - offsetof(type, member)) ; })
It's easier to review patches when they don't make unrelated aesthetic
changes...
-Crystal
Powered by blists - more mailing lists