netdev - Re: [PATCH iproute2-next v4 3/3] dpll: Add dpll command

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251116090926.130f3b9e@phoenix>
Date: Sun, 16 Nov 2025 09:09:26 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Petr Oros <poros@...hat.com>
Cc: netdev@...r.kernel.org, dsahern@...nel.org, ivecera@...hat.com,
 jiri@...nulli.us, Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH iproute2-next v4 3/3] dpll: Add dpll command

On Sun, 16 Nov 2025 00:33:41 +0100
Petr Oros <poros@...hat.com> wrote:

> +	/* Setup signal handler for graceful exit */
> +	memset(&sa, 0, sizeof(sa));

Personal preference, but I like initialization vs memset.
To be pedantic use:
	sigemptyset(&sa.sa_mask);

> +	sa.sa_handler = monitor_sig_handler;
> +	sigaction(SIGINT, &sa, NULL);
> +	sigaction(SIGTERM, &sa, NULL);
> +

Current code is good enough, no need to change.

If you are going to use signal for exit, why not use signalfd() which
avoids lots of problems with interrupts in the middle of the loop.

Checkpatch has some advice, most of it is not applicable but probably
want to look at:

WARNING: Missing a blank line after declarations
#1146: FILE: dpll/dpll.c:554:
+	bool need_nl = true;
+	if (argc > 0 && strcmp(argv[0], "help") == 0)

WARNING: Missing a blank line after declarations
#1725: FILE: dpll/dpll.c:1133:
+		struct nlattr *tb_parent[DPLL_A_PIN_MAX + 1] = {};
+		mnl_attr_parse_nested(ctx->entries[i], attr_pin_cb, tb_parent);

WARNING: Missing a blank line after declarations
#1761: FILE: dpll/dpll.c:1169:
+		struct nlattr *tb_parent[DPLL_A_PIN_MAX + 1] = {};
+		mnl_attr_parse_nested(ctx->entries[i], attr_pin_cb, tb_parent);

WARNING: Missing a blank line after declarations
#1790: FILE: dpll/dpll.c:1198:
+		struct nlattr *tb_ref[DPLL_A_PIN_MAX + 1] = {};
+		mnl_attr_parse_nested(ctx->entries[i], attr_pin_cb, tb_ref);


WARNING: braces {} are not necessary for single statement blocks
#2480: FILE: dpll/dpll.c:1888:
+	if (json) {
+		open_json_array(PRINT_JSON, "monitor");
+	}

WARNING: Block comments use a trailing */ on a separate line
#2508: FILE: dpll/dpll.c:1916:
+			 * If monitor_running is false, we're shutting down gracefully. */

WARNING: braces {} are not necessary for single statement blocks
#2516: FILE: dpll/dpll.c:1924:
+	if (json) {
+		close_json_array(PRINT_JSON, NULL);
+	}