[libvirt] [PATCH 2/2] Try stdin for input when no file is specified

[Eric Blake]

On 06/13/2011 09:39 PM, Michael Williams wrote:
> Modify virFileReadAll to check for redirected stdin input when
> no file is specified.  This means that not every file argument
> needs to be required.
> 
> Signed-off-by: Michael Williams <mspacex at gmail.com>
> ---
>  src/util/util.c |   10 +++++-
>  tools/virsh.c   |   99
> +++++++++++++++++++++++++++++++++++--------------------
>  2 files changed, 72 insertions(+), 37 deletions(-)
> 
> diff --git a/src/util/util.c b/src/util/util.c
> index 554d68e..84b3ae5 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -445,7 +445,15 @@ int virFileReadAll(const char *path, int maxlen,
> char **buf)
>  {
>      int fd;
>  -    if (strcmp(path,"-") == 0)
> +    if (!path) {

Your mailer uses odd spacing.

> +        if (isatty(fileno(stdin))) {

Same comments about using STDIN_FILENO.

I'm debating whether this line belongs here, or whether it belongs
better in virsh.c.  That is, I don't know whether this is a
virsh-specific feature, or whether all of the libvirt library should
behave like this.

I'm also wondering if the tty check is correct; it seems like:

'virsh define'

should be able to read from stdin (it is a batch run rather than
interactive, but has just one command, so it is okay if that command
consumes stdin); whereas:

'virsh' followed by 'define' must not consume stdin, whether or not
stdin is a tty (that is, interactive mode must explicitly request "-" as
the filename)

and:

'virsh "define; list"'

should not consume stdin (batch mode with more than one command, and the
first command should not need to look ahead to see whether subsequent
commands might want to use stdin).

I'm not even sure if stdin being a tty is the right check.

> +            virReportSystemError(EINVAL, "%s", _("Missing <file>
> argument"));
> +            return -1;
> +	} else
> +	    path = "-";
> +    }
> +
> +    if (strcmp(path,"-") == 0)      	fd = fileno(stdin);

Formatting - this should be two lines.

>  @@ -1323,8 +1325,10 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd)
>      if (!vshConnectionUsability(ctl, ctl->conn))
>          return false;
>  -    if (vshCommandOptString(cmd, "file", &from) <= 0)
> +    if (vshCommandOptString(cmd, "file", &from) < 0) {
> +        vshError(ctl, "%s", _("malformed xml argument"));

Not quite the right message; "malformed xml" implies that the file
existed and was read but could not be parsed.  Really, the error here is
that someone called --file '' (that is, the empty string), so a better
error is "empty xml argument".

But overall, the idea is quite nice.

-- 
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/20110613/cd98be02/attachment-0001.sig>