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

[Richard W.M. Jones]

On Mon, Sep 24, 2012 at 02:07:36PM -0600, Eric Blake wrote:
> On 09/24/2012 01:53 PM, Richard W.M. Jones wrote:
> 
> >>> +    /* 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.
> 
> Ah, but glibc is open source, so we can check for ourselves:
> 
> The naive C fallback when no .S is present is highly unoptimized.  From
> glibcc/string/strcspn.c:
> 
> size_t
> strcspn (s, reject)
>      const char *s;
>      const char *reject;
> {
>   size_t count = 0;
> 
>   while (*s != '\0')
>     if (strchr (reject, *s++) == NULL)
>       ++count;
>     else
>       return count;
> 
>   return count;
> }
> 
> and even in the .S optimized versions, there's still no shortcuts taken
> for a one-character reject (possibly worth filing a BZ about the missed
> optimization, though).  From glibc/sysdeps/x86_64/strcspn.S:
>         /* First we create a table with flags for all possible characters.
>            For the ASCII (7bit/8bit) or ISO-8859-X character sets which are
>            supported by the C string functions we have 256 characters.
>            Before inserting marks for the stop characters we clear the whole
>            table.  */
>         movq %rdi, %r8                  /* Save value.  */
>         subq $256, %rsp                 /* Make space for 256 bytes.  */
>         cfi_adjust_cfa_offset(256)
>         movl $32,  %ecx                 /* 32*8 bytes = 256 bytes.  */
>         movq %rsp, %rdi
>         xorl %eax, %eax                 /* We store 0s.  */
> ...
> 
> That is, you are definitely wasting time pre-computing the reject table,
> compared to doing a strchr() for the one rejection.

Possibly, if we weren't just about to spend 100000 cycles doing a fork.
Now, a project to rewrite all Python utilities in Linux in a compiled
high-level language like OCaml, there's something I could get behind :-)

-*-

Unfortunately this patch does not fix the bug, but it now
fails in a different, and stranger way:

libvir:  error : libvirtd quit during handshake: Input/output error

In this code:

        if ((rv = saferead(cmd->handshakeNotify[0], &c,
                           sizeof(c))) != sizeof(c)) {
            if (rv < 0)
                virReportSystemError(errno, "%s",
                                     _("Unable to wait on parent process"));
            else
                virReportSystemError(EIO, "%s",
                                     _("libvirtd quit during handshake"));
            return -1;
        }

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org