netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Michal Kubecek <mkubecek@suse.cz>,
	netdev <netdev@vger.kernel.org>, Chris Healy <cphealy@gmail.com>
Subject: Re: [ethtool v2 4/6] Add --json command line argument parsing
Date: Thu, 25 Jun 2020 16:13:59 +0200	[thread overview]
Message-ID: <20200625141359.GL442307@lunn.ch> (raw)
In-Reply-To: <20200624223258.140f6cad@hermes.lan>

On Wed, Jun 24, 2020 at 10:32:58PM -0700, Stephen Hemminger wrote:
> On Thu, 25 Jun 2020 02:12:42 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > diff --git a/internal.h b/internal.h
> > index edb07bd..7135140 100644
> > --- a/internal.h
> > +++ b/internal.h
> > @@ -23,6 +23,8 @@
> >  #include <sys/ioctl.h>
> >  #include <net/if.h>
> >  
> > +#include "json_writer.h"
> > +
> >  #define maybe_unused __attribute__((__unused__))
> >  
> >  /* internal for netlink interface */
> > @@ -221,6 +223,8 @@ struct cmd_context {
> >  	int argc;		/* number of arguments to the sub-command */
> >  	char **argp;		/* arguments to the sub-command */
> >  	unsigned long debug;	/* debugging mask */
> > +	bool json;		/* Output JSON, if supported */
> > +	json_writer_t *jw;      /* JSON writer instance */
> 
> You can avoid the boolean by just checking for NULL jw variable.

Hi Stephen

It is a while since i wrote this code, but i think i considered
that. The problem is, only a few commands support json output. I could
call json_new() unconditional of if the command actually support json
or not, that is not a problem. But then json_destory() should also be
unconditionally called. And that does fputs("\n", self->out); So you
end up with an extra blank line.

Using the boolean allows me to defer json_new()/json_destroy() into
the actual commands which supports json.

       Andrew

  reply	other threads:[~2020-06-25 14:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-25  0:12 [ethtool v2 0/6] ethtool(1) cable test support Andrew Lunn
2020-06-25  0:12 ` [ethtool v2 1/6] Add " Andrew Lunn
2020-06-25  0:12 ` [ethtool v2 2/6] Add cable test TDR support Andrew Lunn
2020-06-25  0:12 ` [ethtool v2 3/6] json_writer: Import the iproute2 helper code for JSON output Andrew Lunn
2020-06-25  5:32   ` Stephen Hemminger
2020-06-25  5:33   ` Stephen Hemminger
2020-06-25  0:12 ` [ethtool v2 4/6] Add --json command line argument parsing Andrew Lunn
2020-06-25  5:32   ` Stephen Hemminger
2020-06-25 14:13     ` Andrew Lunn [this message]
2020-06-25  0:12 ` [ethtool v2 5/6] ethtool.8.in: Document the cable test commands Andrew Lunn
2020-06-25  0:12 ` [ethtool v2 6/6] ethtool.8.in: Add --json option Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200625141359.GL442307@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=cphealy@gmail.com \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).