[ovs-dev] [PATCH v2 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

[Ginwala, Aliasgar]

Yeah makes sense since we are doing null check in command-line. Thanks for the review and suggestions. Sent v3 https://patchwork.ozlabs.org/patch/1184436/ PTAL.
________________________________
From: Ben Pfaff <blp at ovn.org>
Sent: Friday, October 25, 2019 11:24 AM
To: amginwal at gmail.com <amginwal at gmail.com>
Cc: dev at openvswitch.org <dev at openvswitch.org>; Ginwala, Aliasgar <aginwala at ebay.com>
Subject: Re: [ovs-dev] [PATCH v2 ovn] ovn-nb/sbctl.c: Use env variables for passing options.

On Tue, Oct 15, 2019 at 06:52:52PM -0700, amginwal at gmail.com wrote:
> From: Aliasgar Ginwala <aginwala at ebay.com>
>
> Add new env variables OVN_NBCTL_OPTIONS and OVN_SBCTL_OPTIONS for
> ovn-nbctl and ovn-sbctl respectively where user can set
> supported ovn-nb/sbctl options using environment variable.
> e.g. OVN_SBCTL_OPTIONS="--db=unix:sb1.ovsdb --no-leader-only"
>
> Signed-off-by: Aliasgar Ginwala <aginwala at ebay.com>
> +    /* Check if options are set via env var. */
> +    char *ctl_options = getenv("OVN_NBCTL_OPTIONS");
> +    char **argv;
> +    int *argcp;
> +    argcp = xmalloc(sizeof(int));
> +    *argcp = argc;
> +    argv = ovs_cmdl_env_parse_all(argcp, argv_,
> +                                  ctl_options);
> +    if (!argv) {
> +        /* This situation should never occur, but... */
> +        ctl_fatal("Unable to read argv");
> +    }
> +    argc = *argcp;
> +    free(argcp);

This seems to me to be more complicated than necessary.  Is the
following sufficient?

argv = ovs_cmdl_env_parse_all(&argc, argv, getenv("OVN_NBCTL_OPTIONS"));

I don't know why the check for null argv is needed.