From: Stephen Hemminger <stephen@networkplumber.org>
To: Petr Oros <poros@redhat.com>
Cc: netdev@vger.kernel.org, dsahern@kernel.org, jiri@resnulli.us,
Ivan Vecera <ivecera@redhat.com>
Subject: Re: [PATCH iproute2-next v2] dpll: Add dpll command
Date: Sun, 9 Nov 2025 10:57:39 -0800 [thread overview]
Message-ID: <20251109105739.55162f32@phoenix> (raw)
In-Reply-To: <20251107173116.96622-1-poros@redhat.com>
On Fri, 7 Nov 2025 18:31:16 +0100
Petr Oros <poros@redhat.com> wrote:
> +
> +/* Macros for printing netlink attributes
> + * These macros combine the common pattern of:
> + *
> + * if (tb[ATTR])
> + * print_xxx(PRINT_ANY, "name", "format", mnl_attr_get_xxx(tb[ATTR]));
> + *
> + * Generic versions with custom format string (_FMT suffix)
> + * Simple versions auto-generate format string: " name: %d\n"
> + */
> +
> +#define DPLL_PR_INT_FMT(tb, attr_id, name, format_str) \
> + do { \
> + if (tb[attr_id]) \
> + print_int(PRINT_ANY, name, format_str, \
> + mnl_attr_get_u32(tb[attr_id])); \
> + } while (0)
> +
> +#define DPLL_PR_UINT_FMT(tb, attr_id, name, format_str) \
> + do { \
> + if (tb[attr_id]) \
> + print_uint(PRINT_ANY, name, format_str, \
> + mnl_attr_get_u32(tb[attr_id])); \
> + } while (0)
> +
> +#define DPLL_PR_U64_FMT(tb, attr_id, name, format_str) \
> + do { \
> + if (tb[attr_id]) \
> + print_lluint(PRINT_ANY, name, format_str, \
> + mnl_attr_get_u64(tb[attr_id])); \
> + } while (0)
> +
> +#define DPLL_PR_STR_FMT(tb, attr_id, name, format_str) \
> + do { \
> + if (tb[attr_id]) \
> + print_string(PRINT_ANY, name, format_str, \
> + mnl_attr_get_str(tb[attr_id])); \
> + } while (0)
> +
> +/* Simple versions with auto-generated format */
> +#define DPLL_PR_INT(tb, attr_id, name) \
> + DPLL_PR_INT_FMT(tb, attr_id, name, " " name ": %d\n")
> +
> +#define DPLL_PR_UINT(tb, attr_id, name) \
> + DPLL_PR_UINT_FMT(tb, attr_id, name, " " name ": %u\n")
> +
> +#define DPLL_PR_U64(tb, attr_id, name) \
> + DPLL_PR_U64_FMT(tb, attr_id, name, " " name ": %" PRIu64 "\n")
> +
> +/* Helper to read signed int (can be s32 or s64 depending on value) */
> +static __s64 mnl_attr_get_sint(const struct nlattr *attr)
> +{
> + if (mnl_attr_get_payload_len(attr) == sizeof(__s32))
> + return *(__s32 *)mnl_attr_get_payload(attr);
> + else
> + return *(__s64 *)mnl_attr_get_payload(attr);
> +}
> +
> +#define DPLL_PR_SINT_FMT(tb, attr_id, name, format_str) \
> + do { \
> + if (tb[attr_id]) \
> + print_s64(PRINT_ANY, name, format_str, \
> + mnl_attr_get_sint(tb[attr_id])); \
> + } while (0)
> +
> +#define DPLL_PR_SINT(tb, attr_id, name) \
> + DPLL_PR_SINT_FMT(tb, attr_id, name, " " name ": %" PRId64 "\n")
> +
> +#define DPLL_PR_STR(tb, attr_id, name) \
> + DPLL_PR_STR_FMT(tb, attr_id, name, " " name ": %s\n")
> +
> +/* Temperature macro - JSON prints raw millidegrees, human prints formatted */
> +#define DPLL_PR_TEMP(tb, attr_id) \
> + do { \
> + if (tb[attr_id]) { \
> + __s32 temp = mnl_attr_get_u32(tb[attr_id]); \
> + if (is_json_context()) { \
> + print_int(PRINT_JSON, "temp", NULL, temp); \
> + } else { \
> + div_t d = div(temp, 1000); \
> + pr_out(" temp: %d.%03d C\n", d.quot, d.rem); \
> + } \
> + } \
> + } while (0)
> +
> +/* Generic version with custom format */
> +#define DPLL_PR_ENUM_STR_FMT(tb, attr_id, name, format_str, name_func) \
> + do { \
> + if (tb[attr_id]) \
> + print_string( \
> + PRINT_ANY, name, format_str, \
> + name_func(mnl_attr_get_u32(tb[attr_id]))); \
> + } while (0)
> +
> +/* Simple version with auto-generated format */
> +#define DPLL_PR_ENUM_STR(tb, attr_id, name, name_func) \
> + DPLL_PR_ENUM_STR_FMT(tb, attr_id, name, " " name ": %s\n", name_func)
> +
> +/* Multi-attr enum printer - handles multiple occurrences of same attribute */
> +#define DPLL_PR_MULTI_ENUM_STR(nlh, attr_id, name, name_func) \
> + do { \
> + struct nlattr *__attr; \
> + bool __first = true; \
> + \
> + if (!nlh) \
> + break; \
> + \
> + mnl_attr_for_each(__attr, nlh, sizeof(struct genlmsghdr)) \
> + { \
> + if (mnl_attr_get_type(__attr) == (attr_id)) { \
> + __u32 __val = mnl_attr_get_u32(__attr); \
> + if (__first) { \
> + if (is_json_context()) { \
> + open_json_array(PRINT_JSON, \
> + name); \
> + } else { \
> + pr_out(" " name ":"); \
> + } \
> + __first = false; \
> + } \
> + if (is_json_context()) { \
> + print_string(PRINT_JSON, NULL, NULL, \
> + name_func(__val)); \
> + } else { \
> + pr_out(" %s", name_func(__val)); \
> + } \
> + } \
> + } \
> + if (__first) \
> + break; \
> + if (is_json_context()) { \
> + close_json_array(PRINT_JSON, NULL); \
> + } else { \
> + pr_out("\n"); \
> + } \
> + } while (0)
> +
Code with large macros is harder to read and harder to maintain.
Why can this not be inline functions?
Why do you need to reinvent yet another netlink parser.
Code that does is_json_context() is more verbose than necessary.
> + if (is_json_context()) { \
> + print_string(PRINT_JSON, NULL, NULL, \
> + name_func(__val)); \
> + } else { \
> + pr_out(" %s", name_func(__val)); \
> + }
Can be replaced by:
print_string(PRINT_ANY, NULL, " %s", name_func(__val));
Please use existing code patterns.
next prev parent reply other threads:[~2025-11-09 18:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-07 17:31 [PATCH iproute2-next v2] dpll: Add dpll command Petr Oros
2025-11-09 10:51 ` Jiri Pirko
2025-11-09 17:42 ` David Ahern
2025-11-10 13:09 ` Petr Oros
2025-11-09 18:57 ` Stephen Hemminger [this message]
2025-11-10 13:17 ` Petr Oros
2025-11-09 18:59 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251109105739.55162f32@phoenix \
--to=stephen@networkplumber.org \
--cc=dsahern@kernel.org \
--cc=ivecera@redhat.com \
--cc=jiri@resnulli.us \
--cc=netdev@vger.kernel.org \
--cc=poros@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).