Re: [PATCH ethtool 1/2] sfpid: Fix JSON output of SFP diagnostics

[Johannes Eigner <johannes.eigner@a-eberle.de>]

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

Am Wed, Oct 22, 2025 at 12:55:45PM +0000 schrieb Danielle Ratson:
> > -----Original Message-----
> > From: Johannes Eigner <johannes.eigner@a-eberle.de>
> > Sent: Tuesday, 21 October 2025 17:00
> > To: netdev@vger.kernel.org
> > Cc: Michal Kubecek <mkubecek@suse.cz>; Danielle Ratson
> > <danieller@nvidia.com>; Stephan Wurm <stephan.wurm@a-eberle.de>;
> > Johannes Eigner <johannes.eigner@a-eberle.de>
> > Subject: [PATCH ethtool 1/2] sfpid: Fix JSON output of SFP diagnostics
> > 
> > Close and delete JSON object only after output of SFP diagnostics so
> > that it is also JSON formatted. If the JSON object is deleted too early,
> > some of the output will not be JSON formatted, resulting in mixed output
> > formats.
> > 
> > Signed-off-by: Johannes Eigner <johannes.eigner@a-eberle.de>
> 
> Hi Johannes,
> 
> Please add a fixes tag.
> 

Will update patches with fixes tags.

> > ---
> >  sfpid.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/sfpid.c b/sfpid.c
> > index 62acb4f..5216ce3 100644
> > --- a/sfpid.c
> > +++ b/sfpid.c
> > @@ -520,8 +520,6 @@ int sff8079_show_all_nl(struct cmd_context *ctx)
> >  	new_json_obj(ctx->json);
> >  	open_json_object(NULL);
> >  	sff8079_show_all_common(buf);
> > -	close_json_object();
> > -	delete_json_obj();
> > 
> >  	/* Finish if A2h page is not present */
> >  	if (!(buf[92] & (1 << 6)))
> > @@ -537,6 +535,8 @@ int sff8079_show_all_nl(struct cmd_context *ctx)
> > 
> >  	sff8472_show_all(buf);
> >  out:
> > +	close_json_object();
> > +	delete_json_obj();
> 
> If sff8079_get_eeprom_page() fails, then close_json_object() and delete_json_object() lines will be called although the object was never opened.
> I think those lines should be after the sff8472_show_all(), but outside the out label.
> 

You are right in case the first sff8079_get_eeprom_page() fails,
close_json_object() and delete_json_object() should not be called. But
for the later goto we need to call close_json_object() and
delete_json_object(). Will add second goto mark to resolve this.

> >  	free(buf);
> > 
> >  	return ret;
> > 
> > --
> > 2.43.0
> 

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4153 bytes --]