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@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