Re: [Qemu-devel] [PATCH v8 3/4] qapi: Use an explicit input file

[Eric Blake]

On 04/02/2014 09:16 AM, Lluís Vilanova wrote:
> Use an explicit input file on the command-line instead of reading from 
> standard input
> 
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---

> +input_file = ""
>  output_dir = ""
>  prefix = ""
>  dispatch_type = "sync"
> @@ -389,6 +391,8 @@ do_h = False
>  for o, a in opts:
>      if o in ("-p", "--prefix"):
>          prefix = a
> +    elif o in ("-i", "--input-file"):
> +        input_file = a
>      elif o in ("-o", "--output-dir"):
>          output_dir = a + "/"
>      elif o in ("-t", "--type"):
> @@ -420,7 +424,7 @@ except os.error, e:
>      if e.errno != errno.EEXIST:
>          raise
>  
> -exprs = parse_schema(sys.stdin)
> +exprs = parse_schema(input_file)

I'm assuming this gives a sane error message if the user forgot -i and
you end up trying parse_schema("")?  But that's a fringe case (after
all, most people are ONLY using the makefile to call this script, which
calls it correctly - the only way to notice the problem is calling this
script by hand), so it shouldn't hold up this patch.

> +++ b/tests/Makefile

>       @diff -q $(SRC_PATH)/$*.out $*.test.out
> +     @# Replace in disk to show file in in diff (in case of differences)

s/in in/in/ ?  Or maybe just rewrite the entire comment to something
simpler:

# Sanitize error messages:

> +     @sed -e "s|$(SRC_PATH)/||g" $*.test.err > $*.test.err.sub
> +     @mv $*.test.err.sub $*.test.err
>       @diff -q $(SRC_PATH)/$*.err $*.test.err

Or for that matter, skip the mv and .sub file altogether:

@sed 's|$(SRC_PATH)/||g' $*.test.err | diff -q $(SRC_PATH/$*.err -

If you decide to just tweak the comment, instead of optimizing the 3
lines into one, I could still live with you adding:

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature