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] [day] [month] [year] [list]

Message-ID: <50ea6f8c-42d1-470d-9198-77e33ef070e4@redhat.com>
Date: Thu, 20 Nov 2025 14:20:18 +0100
From: Petr Oros <poros@...hat.com>
To: Stephen Hemminger <stephen@...workplumber.org>
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 11/16/25 18:09, Stephen Hemminger wrote:
> 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);
fixed in v5
>
>> +	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.
I wasn’t aware of signalfd(). After looking into it, I like the approach
  and have rewritten the code to use it in v5.
>
> Checkpatch has some advice, most of it is not applicable but probably
> want to look at:
fixed in v5.
Many thanks for the constructive feedback; I want the code to be the 
best it can be.
>
> 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);
> +	}
>
>