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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list