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

From: Simon Horman
Date: Wed Aug 21 2024 - 15:10:17 EST


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

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;
> +}

...