|SUCCESS| app/test-pmd: terminate process on second signal

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163310

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-27

I'll analyze this DPDK patch for correctness bugs, coding style, and guideline violations.

commit f1d0993e034e (referenced)
Author: Stephen Hemminger <stephen at networkplumber.org>
app/test-pmd: terminate process on second signal

The patch modifies signal handling in testpmd to prevent use-after-free
when a second signal arrives during shutdown. It adds SA_RESETHAND to
reset signal disposition to SIG_DFL after first delivery.

> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index e2569d9e30..d93c696057 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -4603,9 +4603,14 @@ main(int argc, char** argv)
>  	signal(SIGINT, signal_handler);
>  	signal(SIGTERM, signal_handler);
>  #else
> -	/* Want read() not to be restarted on signal */
> +	/*
> +	 * Do not restart read() on signal (no SA_RESTART), and reset
> +	 * to SIG_DFL after first delivery so a second SIGINT/SIGTERM
> +	 * terminates instead of re-entering the shutdown path.
> +	 */
>  	struct sigaction action = {
>  		.sa_handler = signal_handler,
> +		.sa_flags = SA_RESETHAND,
>  	};

Does adding SA_RESETHAND create an inconsistency with the Windows
behavior? The commit message states that Windows C runtime's signal()
already resets the disposition, so this makes the non-Windows path
consistent. However, should the Windows path (lines 4602-4603) also be
considered for similar protection? While Windows signal() auto-resets,
does signal_handler()->prompt_exit()->cmdline_stdin_exit() have the same
use-after-free risk on Windows if a second signal arrives after the
reset but before the process exits?

In signal_handler(), is testpmd_cl checked for NULL before dereferencing
in prompt_exit()? The use-after-free scenario described suggests
testpmd_cl becomes invalid after cmdline_stdin_exit(), so adding a NULL
check in signal_handler() or prompt_exit() might provide defense-in-
depth alongside SA_RESETHAND.

Does the sigaction initialization properly zero sigemask? The struct
initialization shown uses designated initializers for sa_handler and
sa_flags, but does not explicitly initialize sa_mask. While C99
guarantees remaining fields are zero-initialized, should sigemptyset()
be called on action.sa_mask for clarity and to follow common sigaction
usage patterns?