Re: [PATCH net-next v3 2/8] selftests/net: Provide test_snprintf() helper

[Simon Horman <horms@kernel.org>]

On Thu, Aug 15, 2024 at 10:32:27PM +0100, Dmitry Safonov via B4 Relay wrote:
> From: Dmitry Safonov <0x7f454c46@gmail.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@gmail.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;
> +}

...