[PATCH ethtool] netlink: fix missing headers in text output

[PATCH ethtool] netlink: fix missing headers in text output

[Jakub Kicinski]

The commit under fixes added a NULL-check which prevents us from
printing text headers. Conversions to add JSON support often use:

  print_string(PRINT_FP, NULL, "some text:\n", NULL);

to print in plain text mode.

Correct output:

  Channel parameters for vpn0:
  Pre-set maximums:
  RX:		n/a
  TX:		n/a
  Other:		n/a
  Combined:	1
  Current hardware settings:
  RX:		n/a
  TX:		n/a
  Other:		n/a
  Combined:	0

With the buggy patch:

  Channel parameters for vpn0:
  RX:		n/a
  TX:		n/a
  Other:		n/a
  Combined:	1
  RX:		n/a
  TX:		n/a
  Other:		n/a
  Combined:	0

Fixes: fd328ccb3cc0 ("json_print: add NULL check before jsonw_string_field() in print_string()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 json_print.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/json_print.c b/json_print.c
index 4f61640392cf..e07c651f477b 100644
--- a/json_print.c
+++ b/json_print.c
@@ -143,10 +143,11 @@ void print_string(enum output_type type,
 	} else if (_IS_FP_CONTEXT(type)) {
 		if (value)
 			fprintf(stdout, fmt, value);
+		else
+			fprintf(stdout, fmt);
 	}
 }
 
-
 /*
  * value's type is bool. When using this function in FP context you can't pass
  * a value to it, you will need to use "is_json_context()" to have different
-- 
2.50.1

Re: [PATCH ethtool] netlink: fix missing headers in text output

[Jakub Kicinski]

On Sat, 12 Jul 2025 07:51:05 -0700 Jakub Kicinski wrote:
> The commit under fixes added a NULL-check which prevents us from
> printing text headers. Conversions to add JSON support often use:

Hi Michal! 
Would you be able to apply this ASAP and cut a release?
AFAICT this bug has made its way to Fedora, at least.
And some scripts try to match on the section text..

Re: [PATCH ethtool] netlink: fix missing headers in text output

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

Dne Tue, Jul 15, 2025 at 06:40:10AM GMT, Jakub Kicinski napsal:
> On Sat, 12 Jul 2025 07:51:05 -0700 Jakub Kicinski wrote:
> > The commit under fixes added a NULL-check which prevents us from
> > printing text headers. Conversions to add JSON support often use:
> 
> Hi Michal! 
> Would you be able to apply this ASAP and cut a release?
> AFAICT this bug has made its way to Fedora, at least.
> And some scripts try to match on the section text..

I'll see if I can do it in the evening. The problem is that I'm on
family vacation (until Saturday) with very poor connectivity. At least
SSH seems to be the only thing that works reasonably well...

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool] netlink: fix missing headers in text output

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to ethtool/ethtool.git (master)
by Michal Kubecek <mkubecek@suse.cz>:

On Sat, 12 Jul 2025 07:51:05 -0700 you wrote:
> The commit under fixes added a NULL-check which prevents us from
> printing text headers. Conversions to add JSON support often use:
> 
>   print_string(PRINT_FP, NULL, "some text:\n", NULL);
> 
> to print in plain text mode.
> 
> [...]

Here is the summary with links:
  - [ethtool] netlink: fix missing headers in text output
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=b70c92866102

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

RE: [PATCH ethtool] netlink: fix missing headers in text output

[Arinzon, David]

> The commit under fixes added a NULL-check which prevents us from
> printing text headers. Conversions to add JSON support often use:
> 
>   print_string(PRINT_FP, NULL, "some text:\n", NULL);
> 
> to print in plain text mode.
> 
> Correct output:
> 
>   Channel parameters for vpn0:
>   Pre-set maximums:
>   RX:           n/a
>   TX:           n/a
>   Other:                n/a
>   Combined:     1
>   Current hardware settings:
>   RX:           n/a
>   TX:           n/a
>   Other:                n/a
>   Combined:     0
> 
> With the buggy patch:
> 
>   Channel parameters for vpn0:
>   RX:           n/a
>   TX:           n/a
>   Other:                n/a
>   Combined:     1
>   RX:           n/a
>   TX:           n/a
>   Other:                n/a
>   Combined:     0
> 
> Fixes: fd328ccb3cc0 ("json_print: add NULL check before jsonw_string_field() in print_string()")
> Signed-off-by: Jakub Kicinski mailto:kuba@kernel.org
> ---
>  json_print.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/json_print.c b/json_print.c
> index 4f61640392cf..e07c651f477b 100644
> --- a/json_print.c
> +++ b/json_print.c
> @@ -143,10 +143,11 @@ void print_string(enum output_type type,
>         } else if (_IS_FP_CONTEXT(type)) {
>                 if (value)
>                         fprintf(stdout, fmt, value);
> +               else
> +                       fprintf(stdout, fmt);
>         }
>  }
> 
> -
>  /*
>   * value's type is bool. When using this function in FP context you can't pass
>   * a value to it, you will need to use "is_json_context()" to have different
> -- 
> 2.50.1

Thanks for identifying the issue and proposing the fix.

Reviewed-by: David Arinzon <mailto:darinzon@amazon.com>