[<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;
> +}
...
Powered by blists - more mailing lists