* [PATCH] net: sfc: avoid format string warning
@ 2026-03-20 15:19 Arnd Bergmann
2026-03-20 18:00 ` Kees Cook
2026-03-20 19:04 ` Edward Cree
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2026-03-20 15:19 UTC (permalink / raw)
To: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Nathan Chancellor, Jeff Garzik,
Ben Hutchings
Cc: Arnd Bergmann, Nick Desaulniers, Bill Wendling, Justin Stitt,
Brett Creeley, Breno Leitao, Kees Cook, netdev, linux-net-drivers,
linux-kernel, llvm
From: Arnd Bergmann <arnd@arndb.de>
Three sfc drivers contain the same *_fill_test() function that takes
a format string argument that gets passed down to snprintf(), producing
a warning with clang-22:
drivers/net/ethernet/sfc/falcon/ethtool.c:227:60: error: diagnostic behavior may be improved by adding
the 'format(printf, 7, 8)' attribute to the declaration of 'ef4_fill_test' [-Werror,-Wmissing-format-attribute]
210 | snprintf(test_str, sizeof(test_str), test_format, test_id);
| ^
drivers/net/ethernet/sfc/falcon/ethtool.c:210:13: note: 'ef4_fill_test' declared here
Rework these to take a varargs based test_format that allows better
type checking and avoids this warning.
Fixes: 3273c2e8c66a ("[netdrvr] sfc: sfc: Add self-test support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/sfc/ethtool_common.c | 28 ++++++++++--------
drivers/net/ethernet/sfc/falcon/ethtool.c | 27 ++++++++++-------
.../net/ethernet/sfc/siena/ethtool_common.c | 29 +++++++++++--------
3 files changed, 49 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index 54f8e4626568..162be23edbe5 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -254,9 +254,10 @@ int efx_ethtool_set_pauseparam(struct net_device *net_dev,
*
* Fill in an individual self-test entry.
*/
-static void efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
- int *test, const char *unit_format, int unit_id,
- const char *test_format, const char *test_id)
+static void __printf(7, 8)
+efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
+ int *test, const char *unit_format, int unit_id,
+ const char *test_format, ...)
{
char unit_str[ETH_GSTRING_LEN], test_str[ETH_GSTRING_LEN];
@@ -266,12 +267,15 @@ static void efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
/* Fill string, if applicable */
if (strings) {
+ va_list args;
if (strchr(unit_format, '%'))
snprintf(unit_str, sizeof(unit_str),
unit_format, unit_id);
else
strcpy(unit_str, unit_format);
- snprintf(test_str, sizeof(test_str), test_format, test_id);
+ va_start(args, test_format);
+ vsnprintf(test_str, sizeof(test_str), test_format, args);
+ va_end(args);
snprintf(strings + test_index * ETH_GSTRING_LEN,
ETH_GSTRING_LEN,
"%-6s %-24s", unit_str, test_str);
@@ -349,28 +353,28 @@ int efx_ethtool_fill_self_tests(struct efx_nic *efx,
enum efx_loopback_mode mode;
efx_fill_test(n++, strings, data, &tests->phy_alive,
- "phy", 0, "alive", NULL);
+ "phy", 0, "alive");
efx_fill_test(n++, strings, data, &tests->nvram,
- "core", 0, "nvram", NULL);
+ "core", 0, "nvram");
efx_fill_test(n++, strings, data, &tests->interrupt,
- "core", 0, "interrupt", NULL);
+ "core", 0, "interrupt");
/* Event queues */
efx_for_each_channel(channel, efx) {
efx_fill_test(n++, strings, data,
&tests->eventq_dma[channel->channel],
EFX_CHANNEL_NAME(channel),
- "eventq.dma", NULL);
+ "eventq.dma");
efx_fill_test(n++, strings, data,
&tests->eventq_int[channel->channel],
EFX_CHANNEL_NAME(channel),
- "eventq.int", NULL);
+ "eventq.int");
}
efx_fill_test(n++, strings, data, &tests->memory,
- "core", 0, "memory", NULL);
+ "core", 0, "memory");
efx_fill_test(n++, strings, data, &tests->registers,
- "core", 0, "registers", NULL);
+ "core", 0, "registers");
for (i = 0; true; ++i) {
const char *name;
@@ -380,7 +384,7 @@ int efx_ethtool_fill_self_tests(struct efx_nic *efx,
if (name == NULL)
break;
- efx_fill_test(n++, strings, data, &tests->phy_ext[i], "phy", 0, name, NULL);
+ efx_fill_test(n++, strings, data, &tests->phy_ext[i], "phy", 0, "%s", name);
}
/* Loopback tests */
diff --git a/drivers/net/ethernet/sfc/falcon/ethtool.c b/drivers/net/ethernet/sfc/falcon/ethtool.c
index 807c587eba3b..1ae8acb8fdc8 100644
--- a/drivers/net/ethernet/sfc/falcon/ethtool.c
+++ b/drivers/net/ethernet/sfc/falcon/ethtool.c
@@ -207,9 +207,10 @@ static void ef4_ethtool_set_msglevel(struct net_device *net_dev, u32 msg_enable)
*
* Fill in an individual self-test entry.
*/
-static void ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
+static void __printf(7, 8)
+ ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
int *test, const char *unit_format, int unit_id,
- const char *test_format, const char *test_id)
+ const char *test_format, ...)
{
char unit_str[ETH_GSTRING_LEN], test_str[ETH_GSTRING_LEN];
@@ -219,12 +220,16 @@ static void ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
/* Fill string, if applicable */
if (strings) {
+ va_list arg;
+
if (strchr(unit_format, '%'))
snprintf(unit_str, sizeof(unit_str),
unit_format, unit_id);
else
strcpy(unit_str, unit_format);
- snprintf(test_str, sizeof(test_str), test_format, test_id);
+ va_start(arg, test_format);
+ vsnprintf(test_str, sizeof(test_str), test_format, arg);
+ va_end(arg);
snprintf(strings + test_index * ETH_GSTRING_LEN,
ETH_GSTRING_LEN,
"%-6s %-24s", unit_str, test_str);
@@ -303,28 +308,28 @@ static int ef4_ethtool_fill_self_tests(struct ef4_nic *efx,
enum ef4_loopback_mode mode;
ef4_fill_test(n++, strings, data, &tests->phy_alive,
- "phy", 0, "alive", NULL);
+ "phy", 0, "alive");
ef4_fill_test(n++, strings, data, &tests->nvram,
- "core", 0, "nvram", NULL);
+ "core", 0, "nvram");
ef4_fill_test(n++, strings, data, &tests->interrupt,
- "core", 0, "interrupt", NULL);
+ "core", 0, "interrupt");
/* Event queues */
ef4_for_each_channel(channel, efx) {
ef4_fill_test(n++, strings, data,
&tests->eventq_dma[channel->channel],
EF4_CHANNEL_NAME(channel),
- "eventq.dma", NULL);
+ "eventq.dma");
ef4_fill_test(n++, strings, data,
&tests->eventq_int[channel->channel],
EF4_CHANNEL_NAME(channel),
- "eventq.int", NULL);
+ "eventq.int");
}
ef4_fill_test(n++, strings, data, &tests->memory,
- "core", 0, "memory", NULL);
+ "core", 0, "memory");
ef4_fill_test(n++, strings, data, &tests->registers,
- "core", 0, "registers", NULL);
+ "core", 0, "registers");
if (efx->phy_op->run_tests != NULL) {
EF4_BUG_ON_PARANOID(efx->phy_op->test_name == NULL);
@@ -338,7 +343,7 @@ static int ef4_ethtool_fill_self_tests(struct ef4_nic *efx,
break;
ef4_fill_test(n++, strings, data, &tests->phy_ext[i],
- "phy", 0, name, NULL);
+ "phy", 0, "%s", name);
}
}
diff --git a/drivers/net/ethernet/sfc/siena/ethtool_common.c b/drivers/net/ethernet/sfc/siena/ethtool_common.c
index 76cbce2b9592..9a8e39c5a113 100644
--- a/drivers/net/ethernet/sfc/siena/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/siena/ethtool_common.c
@@ -206,9 +206,9 @@ int efx_siena_ethtool_set_pauseparam(struct net_device *net_dev,
*
* Fill in an individual self-test entry.
*/
-static void efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
- int *test, const char *unit_format, int unit_id,
- const char *test_format, const char *test_id)
+static void __printf(7, 8)
+efx_fill_test(unsigned int test_index, u8 *strings, u64 *data, int *test,
+ const char *unit_format, int unit_id, const char *test_format, ...)
{
char unit_str[ETH_GSTRING_LEN], test_str[ETH_GSTRING_LEN];
@@ -218,12 +218,16 @@ static void efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
/* Fill string, if applicable */
if (strings) {
+ va_list args;
+
if (strchr(unit_format, '%'))
snprintf(unit_str, sizeof(unit_str),
unit_format, unit_id);
else
strcpy(unit_str, unit_format);
- snprintf(test_str, sizeof(test_str), test_format, test_id);
+ va_start(args, test_format);
+ vsnprintf(test_str, sizeof(test_str), test_format, args);
+ va_end(args);
snprintf(strings + test_index * ETH_GSTRING_LEN,
ETH_GSTRING_LEN,
"%-6s %-24s", unit_str, test_str);
@@ -301,28 +305,28 @@ static int efx_ethtool_fill_self_tests(struct efx_nic *efx,
enum efx_loopback_mode mode;
efx_fill_test(n++, strings, data, &tests->phy_alive,
- "phy", 0, "alive", NULL);
+ "phy", 0, "alive");
efx_fill_test(n++, strings, data, &tests->nvram,
- "core", 0, "nvram", NULL);
+ "core", 0, "nvram");
efx_fill_test(n++, strings, data, &tests->interrupt,
- "core", 0, "interrupt", NULL);
+ "core", 0, "interrupt");
/* Event queues */
efx_for_each_channel(channel, efx) {
efx_fill_test(n++, strings, data,
&tests->eventq_dma[channel->channel],
EFX_CHANNEL_NAME(channel),
- "eventq.dma", NULL);
+ "eventq.dma");
efx_fill_test(n++, strings, data,
&tests->eventq_int[channel->channel],
EFX_CHANNEL_NAME(channel),
- "eventq.int", NULL);
+ "eventq.int");
}
efx_fill_test(n++, strings, data, &tests->memory,
- "core", 0, "memory", NULL);
+ "core", 0, "memory");
efx_fill_test(n++, strings, data, &tests->registers,
- "core", 0, "registers", NULL);
+ "core", 0, "registers");
for (i = 0; true; ++i) {
const char *name;
@@ -332,7 +336,8 @@ static int efx_ethtool_fill_self_tests(struct efx_nic *efx,
if (name == NULL)
break;
- efx_fill_test(n++, strings, data, &tests->phy_ext[i], "phy", 0, name, NULL);
+ efx_fill_test(n++, strings, data, &tests->phy_ext[i], "phy",
+ 0, "%s", name);
}
/* Loopback tests */
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: sfc: avoid format string warning
2026-03-20 15:19 [PATCH] net: sfc: avoid format string warning Arnd Bergmann
@ 2026-03-20 18:00 ` Kees Cook
2026-03-20 19:04 ` Edward Cree
1 sibling, 0 replies; 6+ messages in thread
From: Kees Cook @ 2026-03-20 18:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Nathan Chancellor, Jeff Garzik,
Ben Hutchings, Arnd Bergmann, Nick Desaulniers, Bill Wendling,
Justin Stitt, Brett Creeley, Breno Leitao, netdev,
linux-net-drivers, linux-kernel, llvm
On Fri, Mar 20, 2026 at 04:19:16PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Three sfc drivers contain the same *_fill_test() function that takes
> a format string argument that gets passed down to snprintf(), producing
> a warning with clang-22:
>
> drivers/net/ethernet/sfc/falcon/ethtool.c:227:60: error: diagnostic behavior may be improved by adding
> the 'format(printf, 7, 8)' attribute to the declaration of 'ef4_fill_test' [-Werror,-Wmissing-format-attribute]
> 210 | snprintf(test_str, sizeof(test_str), test_format, test_id);
> | ^
> drivers/net/ethernet/sfc/falcon/ethtool.c:210:13: note: 'ef4_fill_test' declared here
>
> Rework these to take a varargs based test_format that allows better
> type checking and avoids this warning.
>
> Fixes: 3273c2e8c66a ("[netdrvr] sfc: sfc: Add self-test support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Nice rework!
Reviewed-by: Kees Cook <kees@kernel.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: sfc: avoid format string warning
2026-03-20 15:19 [PATCH] net: sfc: avoid format string warning Arnd Bergmann
2026-03-20 18:00 ` Kees Cook
@ 2026-03-20 19:04 ` Edward Cree
2026-03-20 20:48 ` Arnd Bergmann
1 sibling, 1 reply; 6+ messages in thread
From: Edward Cree @ 2026-03-20 19:04 UTC (permalink / raw)
To: Arnd Bergmann, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Nathan Chancellor, Jeff Garzik,
Ben Hutchings
Cc: Arnd Bergmann, Nick Desaulniers, Bill Wendling, Justin Stitt,
Brett Creeley, Breno Leitao, Kees Cook, netdev, linux-net-drivers,
linux-kernel, llvm
On 20/03/2026 15:19, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Three sfc drivers contain the same *_fill_test() function that takes
> a format string argument that gets passed down to snprintf(), producing
> a warning with clang-22:
>
> drivers/net/ethernet/sfc/falcon/ethtool.c:227:60: error: diagnostic behavior may be improved by adding
> the 'format(printf, 7, 8)' attribute to the declaration of 'ef4_fill_test' [-Werror,-Wmissing-format-attribute]
> 210 | snprintf(test_str, sizeof(test_str), test_format, test_id);
> | ^
> drivers/net/ethernet/sfc/falcon/ethtool.c:210:13: note: 'ef4_fill_test' declared here
>
> Rework these to take a varargs based test_format that allows better
> type checking and avoids this warning.
>
> Fixes: 3273c2e8c66a ("[netdrvr] sfc: sfc: Add self-test support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
I'm not convinced this is an improvement. The function takes *two* printf
format strings (unit_format and test_format), and this only annotates and
varargs-ifies one of them (and doesn't harden the unit_format, which has
the considerably more worrying-looking "if it has a % in, assume it takes
precisely one int argument").
So it may make things 'look' hardened enough to satisfy the compiler
warning, but that just hides the unsafety and might sucker future
developers into thinking "oh, the compiler knows about the format strings
so I don't need to check it by hand".
I'm not sure what a proper fix would look like — possibly a rewrite adding
a separate efx_fill_test_perq() function for the cases with a nontrivial
unit_format, baking in the knowledge that the format is always "foo%d"
for some constant string foo and thus we don't actually need to pass in
an arbitrary format string.
> -static void efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
> - int *test, const char *unit_format, int unit_id,
> - const char *test_format, const char *test_id)
> +static void __printf(7, 8)
> +efx_fill_test(unsigned int test_index, u8 *strings, u64 *data,
I don't like splitting the specifiers/return type from the function name
like this. It's not Linux style[1].
(From a quick poke around the tree, there seems to be less consensus on
where the __printf attribute goes — some put it on its own line before
`static`, some would put it between `static` and `void`… sigh.)
-Ed
[1]: https://lore.kernel.org/all/1054519757.161606@palladium.transmeta.com/T/#u
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: sfc: avoid format string warning
2026-03-20 19:04 ` Edward Cree
@ 2026-03-20 20:48 ` Arnd Bergmann
2026-03-25 0:22 ` Edward Cree
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2026-03-20 20:48 UTC (permalink / raw)
To: Edward Cree, Arnd Bergmann, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nathan Chancellor,
Jeff Garzik, Ben Hutchings
Cc: Nick Desaulniers, Bill Wendling, Justin Stitt, Brett Creeley,
Breno Leitao, Kees Cook, Netdev, linux-net-drivers, linux-kernel,
llvm
On Fri, Mar 20, 2026, at 20:04, Edward Cree wrote:
> On 20/03/2026 15:19, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Three sfc drivers contain the same *_fill_test() function that takes
>> a format string argument that gets passed down to snprintf(), producing
>> a warning with clang-22:
>>
>> drivers/net/ethernet/sfc/falcon/ethtool.c:227:60: error: diagnostic behavior may be improved by adding
>> the 'format(printf, 7, 8)' attribute to the declaration of 'ef4_fill_test' [-Werror,-Wmissing-format-attribute]
>> 210 | snprintf(test_str, sizeof(test_str), test_format, test_id);
>> | ^
>> drivers/net/ethernet/sfc/falcon/ethtool.c:210:13: note: 'ef4_fill_test' declared here
>>
>> Rework these to take a varargs based test_format that allows better
>> type checking and avoids this warning.
>>
>> Fixes: 3273c2e8c66a ("[netdrvr] sfc: sfc: Add self-test support")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> I'm not convinced this is an improvement. The function takes *two* printf
> format strings (unit_format and test_format), and this only annotates and
> varargs-ifies one of them (and doesn't harden the unit_format, which has
> the considerably more worrying-looking "if it has a % in, assume it takes
> precisely one int argument").
Right, I took a bit of a shortcut there. How about changing it
so the other format string is just a name that gets joined with the
number?
diff --git a/drivers/net/ethernet/sfc/falcon/ethtool.c b/drivers/net/ethernet/sfc/falcon/ethtool.c
index 1ae8acb8fdc8..a0064eadff41 100644
--- a/drivers/net/ethernet/sfc/falcon/ethtool.c
+++ b/drivers/net/ethernet/sfc/falcon/ethtool.c
@@ -200,7 +200,7 @@ static void ef4_ethtool_set_msglevel(struct net_device *net_dev, u32 msg_enable)
* @strings: Ethtool strings, or %NULL
* @data: Ethtool test results, or %NULL
* @test: Pointer to test result (used only if data != %NULL)
- * @unit_format: Unit name format (e.g. "chan\%d")
+ * @unit_name: Unit name format (e.g. "chan\%d")
* @unit_id: Unit id (e.g. 0 for "chan0")
* @test_format: Test name format (e.g. "loopback.\%s.tx.sent")
* @test_id: Test id (e.g. "PHYXS" for "loopback.PHYXS.tx_sent")
@@ -208,9 +208,9 @@ static void ef4_ethtool_set_msglevel(struct net_device *net_dev, u32 msg_enable)
* Fill in an individual self-test entry.
*/
static void __printf(7, 8)
- ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
- int *test, const char *unit_format, int unit_id,
- const char *test_format, ...)
+ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
+ int *test, const char *unit_name, int unit_id,
+ const char *test_format, ...)
{
char unit_str[ETH_GSTRING_LEN], test_str[ETH_GSTRING_LEN];
@@ -222,11 +222,11 @@ static void __printf(7, 8)
if (strings) {
va_list arg;
- if (strchr(unit_format, '%'))
+ if (unit_id >= 0)
snprintf(unit_str, sizeof(unit_str),
- unit_format, unit_id);
+ "%s%d", unit_name, unit_id);
else
- strcpy(unit_str, unit_format);
+ strcpy(unit_str, unit_name);
va_start(arg, test_format);
vsnprintf(test_str, sizeof(test_str), test_format, arg);
va_end(arg);
@@ -236,9 +236,9 @@ static void __printf(7, 8)
}
}
-#define EF4_CHANNEL_NAME(_channel) "chan%d", _channel->channel
-#define EF4_TX_QUEUE_NAME(_tx_queue) "txq%d", _tx_queue->queue
-#define EF4_RX_QUEUE_NAME(_rx_queue) "rxq%d", _rx_queue->queue
+#define EF4_CHANNEL_NAME(_channel) "chan", _channel->channel
+#define EF4_TX_QUEUE_NAME(_tx_queue) "txq", _tx_queue->queue
+#define EF4_RX_QUEUE_NAME(_rx_queue) "rxq", _rx_queue->queue
#define EF4_LOOPBACK_NAME(_mode, _counter) \
"loopback.%s." _counter, STRING_TABLE_LOOKUP(_mode, ef4_loopback_mode)
@@ -276,11 +276,11 @@ static int ef4_fill_loopback_test(struct ef4_nic *efx,
}
ef4_fill_test(test_index++, strings, data,
&lb_tests->rx_good,
- "rx", 0,
+ "rx", -1,
EF4_LOOPBACK_NAME(mode, "rx_good"));
ef4_fill_test(test_index++, strings, data,
&lb_tests->rx_bad,
- "rx", 0,
+ "rx", -1,
EF4_LOOPBACK_NAME(mode, "rx_bad"));
return test_index;
@@ -308,11 +308,11 @@ static int ef4_ethtool_fill_self_tests(struct ef4_nic *efx,
enum ef4_loopback_mode mode;
ef4_fill_test(n++, strings, data, &tests->phy_alive,
- "phy", 0, "alive");
+ "phy", -1, "alive");
ef4_fill_test(n++, strings, data, &tests->nvram,
- "core", 0, "nvram");
+ "core", -1, "nvram");
ef4_fill_test(n++, strings, data, &tests->interrupt,
- "core", 0, "interrupt");
+ "core", -1, "interrupt");
/* Event queues */
ef4_for_each_channel(channel, efx) {
@@ -327,9 +327,9 @@ static int ef4_ethtool_fill_self_tests(struct ef4_nic *efx,
}
ef4_fill_test(n++, strings, data, &tests->memory,
- "core", 0, "memory");
+ "core", -1, "memory");
ef4_fill_test(n++, strings, data, &tests->registers,
- "core", 0, "registers");
+ "core", -1, "registers");
if (efx->phy_op->run_tests != NULL) {
EF4_BUG_ON_PARANOID(efx->phy_op->test_name == NULL);
@@ -343,7 +343,7 @@ static int ef4_ethtool_fill_self_tests(struct ef4_nic *efx,
break;
ef4_fill_test(n++, strings, data, &tests->phy_ext[i],
- "phy", 0, "%s", name);
+ "phy", -1, "%s", name);
}
}
> I don't like splitting the specifiers/return type from the function name
> like this. It's not Linux style[1].
> (From a quick poke around the tree, there seems to be less consensus on
> where the __printf attribute goes — some put it on its own line before
> `static`, some would put it between `static` and `void`… sigh.)
>
> [1]: https://lore.kernel.org/all/1054519757.161606@palladium.transmeta.com/T/#u
There are not a lot of good options here, and splitting the line seems
better than an overlong line to me. I don't really have a strong opinion
on where the __printf attribute should go either, but I do see that
after the return type is probably the least common, so I'll change
that. How about having the specifiers on one line and
the type in front of the name? that seems faily common.
static __printf(7, 8)
void ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
Arnd
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: sfc: avoid format string warning
2026-03-20 20:48 ` Arnd Bergmann
@ 2026-03-25 0:22 ` Edward Cree
2026-03-25 13:48 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Edward Cree @ 2026-03-25 0:22 UTC (permalink / raw)
To: Arnd Bergmann, Arnd Bergmann, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nathan Chancellor,
Jeff Garzik, Ben Hutchings
Cc: Nick Desaulniers, Bill Wendling, Justin Stitt, Brett Creeley,
Breno Leitao, Kees Cook, Netdev, linux-net-drivers, linux-kernel,
llvm
On 20/03/2026 20:48, Arnd Bergmann wrote:
> Right, I took a bit of a shortcut there. How about changing it
> so the other format string is just a name that gets joined with the
> number?
That would work, but all those -1s are a bit unsightly. Like I said,
I'd rather have two different efx_fill_test() functions, one which
takes a unit_id and one which doesn't.
(Also you need to change the kdoc description of unit_name.)
> There are not a lot of good options here, and splitting the line seems
> better than an overlong line to me. I don't really have a strong opinion
> on where the __printf attribute should go either, but I do see that
> after the return type is probably the least common, so I'll change
> that. How about having the specifiers on one line and
> the type in front of the name? that seems faily common.
>
> static __printf(7, 8)
> void ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
I'd rather just have
static void __printf(7, 8) ef4_fill_test(unsigned int test_index,
Admittedly that puts the ( further over so fewer params fit on each
line, but I definitely feel the function name should always appear
on the first line of the definition.
-ed
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: sfc: avoid format string warning
2026-03-25 0:22 ` Edward Cree
@ 2026-03-25 13:48 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2026-03-25 13:48 UTC (permalink / raw)
To: Edward Cree, Arnd Bergmann, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Nathan Chancellor,
Jeff Garzik, Ben Hutchings
Cc: Nick Desaulniers, Bill Wendling, Justin Stitt, Brett Creeley,
Breno Leitao, Kees Cook, Netdev, linux-net-drivers, linux-kernel,
llvm
On Wed, Mar 25, 2026, at 01:22, Edward Cree wrote:
> On 20/03/2026 20:48, Arnd Bergmann wrote:
>
> That would work, but all those -1s are a bit unsightly. Like I said,
> I'd rather have two different efx_fill_test() functions, one which
> takes a unit_id and one which doesn't.
> (Also you need to change the kdoc description of unit_name.)
I sent a new version now, tried your suggestion but ended up with
yet another variation where I preformat the string in the two
cases (per driver) that actually use a nontrivial format. This
ended up simpler both four the source and compiled form.
>> There are not a lot of good options here, and splitting the line seems
>> better than an overlong line to me. I don't really have a strong opinion
>> on where the __printf attribute should go either, but I do see that
>> after the return type is probably the least common, so I'll change
>> that. How about having the specifiers on one line and
>> the type in front of the name? that seems faily common.
>>
>> static __printf(7, 8)
>> void ef4_fill_test(unsigned int test_index, u8 *strings, u64 *data,
>
> I'd rather just have
> static void __printf(7, 8) ef4_fill_test(unsigned int test_index,
Done.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-25 13:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 15:19 [PATCH] net: sfc: avoid format string warning Arnd Bergmann
2026-03-20 18:00 ` Kees Cook
2026-03-20 19:04 ` Edward Cree
2026-03-20 20:48 ` Arnd Bergmann
2026-03-25 0:22 ` Edward Cree
2026-03-25 13:48 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox