[libvirt] [PATCH 1/2] build: avoid close, system

[Eric Blake]

On 01/28/2011 03:39 PM, Matthias Bolte wrote:
>> +    if (!editor)
>> +        editor = "vi"; /* could be cruel & default to ed(1) here */
> 
> When VISUAL and EDITOR isn't set we fallback to vi here, but ...
> 
>> +        cmd = virCommandNewArgList("sh", "-c", NULL);
>> +        virCommandAddArgFormat(cmd, "%s %s", editor, filename);
>> +    } else {
>> +        cmd = virCommandNewArgList("editor", filename, NULL);

AARGH - stupid typo.  That should be 'editor', not '"editor"'.  (Can you
tell that I tested the patch with EDITOR='emacs -nw' in my environment,
and not with EDITOR unset or a simple shell word?)

> 
> Anyway, that's minor and doesn't affect my ACK.

It does affect me applying the patch, though.

Ooh, one other bug.  system() shares stdin with the child process
(important if you invoke 'virsh' interactively, and your editor wants to
reuse the same stdin as virsh for the duration of virsh being blocked).
 But virCommand defaults to redirecting stdin from /dev/null (which
would make such an editor a nop, because stdin would be at EOF).  v2
coming up.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110128/0dd7a7d6/attachment-0001.sig>