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

[Stephen Hemminger <stephen@networkplumber.org>]

On Sun, 16 Nov 2025 00:33:41 +0100
Petr Oros <poros@redhat.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);
+	}