| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251109105739.55162f32@phoenix>
Date: Sun, 9 Nov 2025 10:57:39 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Petr Oros <poros@...hat.com>
Cc: netdev@...r.kernel.org, dsahern@...nel.org, jiri@...nulli.us, Ivan
Vecera <ivecera@...hat.com>
Subject: Re: [PATCH iproute2-next v2] dpll: Add dpll command
On Fri, 7 Nov 2025 18:31:16 +0100
Petr Oros <poros@...hat.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.
Powered by blists - more mailing lists