Re: [PATCH mptcp-next v2] selftests: mptcp: print stats before socket closure

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Shell command substitution inside single quotes prevents evaluation in system() call
--

--- Patch [1]: [PATCH mptcp-next v2] selftests: mptcp: print stats before socket closure ---
commit 3f1a40cd885883e270f0fd3544c47bf2dc166581
Author: Matthieu Baerts (NGI0) <[email protected]>

selftests: mptcp: print stats before socket closure

In case of a poll timeout, this commit invokes 'ss' and 'nstat' via
system() to print debug stats before exiting mptcp_connect.

Link: https://patch.msgid.link/[email protected]

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index ea4cb6c1bd5e2..e906abcd54096 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -210,6 +210,19 @@ static void xgetaddrinfo(const char *node, const char *service,
>  	}
>  }
>  
> +static void print_err_stats(void)
> +{
> +	char cmd[128];
> +
> +	snprintf(cmd, sizeof(cmd), "ss -Menitam -o '%cport = :%s' >&2",
> +		 listen_mode ? 's' : 'd', cfg_port);
> +
> +	fprintf(stderr, "socket stats before socket closure:\n");
> +	(void)!system(cmd);
> +	(void)!system("NSTAT_HISTORY='/tmp/$(ip netns identify).nstat' "
> +		      "nstat -s '*Tcp*' >&2");

[Severity: Medium]
Will this shell command substitution be evaluated properly?

Because the path is enclosed in single quotes, it appears NSTAT_HISTORY
will be set to the literal string '/tmp/$(ip netns identify).nstat' instead
of being evaluated by the shell.

If the shell does not evaluate the command substitution, could this cause
nstat to look for a non-existent history file and print absolute stats
instead of the expected differential stats?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1