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 <[email protected]> Sent: Friday, October 25, 2019 11:24 AM To: [email protected] <[email protected]> Cc: [email protected] <[email protected]>; Ginwala, Aliasgar <[email protected]> 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, [email protected] wrote: > From: Aliasgar Ginwala <[email protected]> > > 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 <[email protected]> > + /* 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
