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