[libvirt] [PATCH v1 2/2] command: Change virCommandAddEnv so it replaces existing environment variables.

[Richard W.M. Jones]

On Mon, Sep 24, 2012 at 01:49:37PM -0600, Eric Blake wrote:
> On 09/24/2012 12:54 PM, Richard W.M. Jones wrote:
> > From: "Richard W.M. Jones" <rjones at redhat.com>
> > 
> > ---
> >  src/util/command.c |   17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/util/command.c b/src/util/command.c
> > index f7d92dd..354e526 100644
> > --- a/src/util/command.c
> > +++ b/src/util/command.c
> > @@ -985,11 +985,26 @@ virCommandNonblockingFDs(virCommandPtr cmd)
> >  }
> >  
> >  /* Add an environment variable to the cmd->env list.  'env' is a
> > - * string like "name=value".
> > + * string like "name=value".  If the named environment variable is
> > + * already set, then it is replaced in the list.
> >   */
> >  static inline void
> >  virCommandAddEnv(virCommandPtr cmd, char *env)
> >  {
> > +    size_t namelen;
> > +    size_t i;
> > +
> > +    /* Search for the name in the existing environment. */
> > +    namelen = strcspn(env, "=");
> 
> Would 'strchr(env, '=') - env' be any more efficient?  But that's a
> micro-optimization, probably not worth worrying about.

I guess I trust glibc or gcc to have these string primitives
optimized better than I could.

> > +    for (i = 0; i < cmd->nenv; ++i) {
> > +        /* +1 because we want to match the '=' character too. */
> > +        if (STREQLEN(cmd->env[i], env, namelen+1)) {
> 
> Coding style - spaces on both sides of '+'.  Or even hoist the +1
> outside of the loop to the original computation of namelen.
> 
> ACK with that fix.

Thanks -- both patches pushed, the second one with the
recommended change.

I will continue with testing to see if this does or doesn't
fix the actual bug.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top