linux-kernel - Re: [PATCH net-next v3 2/8] selftests/net: Provide test

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: <20240821191004.GF2164@kernel.org>
Date: Wed, 21 Aug 2024 20:10:04 +0100
From: Simon Horman <horms@...nel.org>
To: 0x7f454c46@...il.com
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Shuah Khan <shuah@...nel.org>,
	Mohammad Nassiri <mnassiri@...na.com>, netdev@...r.kernel.org,
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/8] selftests/net: Provide test_snprintf()
 helper

On Thu, Aug 15, 2024 at 10:32:27PM +0100, Dmitry Safonov via B4 Relay wrote:
> From: Dmitry Safonov <0x7f454c46@...il.com>
> 
> Instead of pre-allocating a fixed-sized buffer of TEST_MSG_BUFFER_SIZE
> and printing into it, call vsnprintf() with str = NULL, which will
> return the needed size of the buffer. This hack is documented in
> man 3 vsnprintf.
> 
> Essentially, in C++ terms, it re-invents std::stringstream, which is
> going to be used to print different tracing paths and formatted strings.
> Use it straight away in __test_print() - which is thread-safe version of
> printing in selftests.
> 
> Signed-off-by: Dmitry Safonov <0x7f454c46@...il.com>

Hi Dmitry,

Some minor nits, as it looks like there will be a v4.

> ---
>  tools/testing/selftests/net/tcp_ao/lib/aolib.h | 59 ++++++++++++++++++++++----
>  1 file changed, 50 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/tcp_ao/lib/aolib.h b/tools/testing/selftests/net/tcp_ao/lib/aolib.h
> index fbc7f6111815..60a63419cabb 100644
> --- a/tools/testing/selftests/net/tcp_ao/lib/aolib.h
> +++ b/tools/testing/selftests/net/tcp_ao/lib/aolib.h
> @@ -37,17 +37,58 @@ extern void __test_xfail(const char *buf);
>  extern void __test_error(const char *buf);
>  extern void __test_skip(const char *buf);
>  
> -__attribute__((__format__(__printf__, 2, 3)))
> -static inline void __test_print(void (*fn)(const char *), const char *fmt, ...)
> +static inline char *test_snprintf(const char *fmt, va_list vargs)
>  {
> -#define TEST_MSG_BUFFER_SIZE 4096
> -	char buf[TEST_MSG_BUFFER_SIZE];
> -	va_list arg;
> +	char *ret = NULL;
> +	size_t size = 0;
> +	va_list tmp;
> +	int n = 0;
>  
> -	va_start(arg, fmt);
> -	vsnprintf(buf, sizeof(buf), fmt, arg);
> -	va_end(arg);
> -	fn(buf);
> +	va_copy(tmp, vargs);
> +	n = vsnprintf(ret, size, fmt, tmp);
> +	if (n < 0)
> +		return NULL;
> +
> +	size = (size_t) n + 1;

nit: I'm not sure this cast is needed.
     But if it is, then the space after ')' should be dropped.

> +	ret = malloc(size);
> +	if (ret == NULL)

nit: if (!ret)

> +		return NULL;
> +
> +	n = vsnprintf(ret, size, fmt, vargs);
> +	if (n < 0 || n > size - 1) {
> +		free(ret);
> +		return NULL;
> +	}
> +	return ret;
> +}

...