[PATCH wayland v3] Add support for direct file reading and writing in wayland-scanner.

[Pekka Paalanen]

On Mon,  2 Mar 2015 16:08:00 +0200
Jussi Pakkanen <jpakkane at gmail.com> wrote:

> Add support for direct file reading and writing in wayland-scanner.
> 
> Signed-off-by: Jussi Pakkanen <jpakkane at gmail.com>
> 
> ---
>  src/scanner.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/src/scanner.c b/src/scanner.c
> index 1f1e59a..efdc69c 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -39,11 +39,12 @@ enum side {
>  static int
>  usage(int ret)
>  {
> -	fprintf(stderr, "usage: ./scanner [client-header|server-header|code]\n");
> +	fprintf(stderr, "usage: ./scanner [client-header|server-header|code]"
> +		" [input_file output_file]\n");
>  	fprintf(stderr, "\n");
>  	fprintf(stderr, "Converts XML protocol descriptions supplied on "
> -			"stdin to client headers,\n"
> -			"server headers, or protocol marshalling code.\n");
> +			"stdin or input file to client\n"
> +			"headers, server headers, or protocol marshalling code.\n");

I don't think this is exactly clear on that the input/output files are
optional, but if you define one, you must define both; otherwise it
uses stdin/out. But I don't mind, it's not a tool people run manually.

>  	exit(ret);
>  }
>  
> @@ -1252,6 +1253,7 @@ int main(int argc, char *argv[])
>  {
>  	struct parse_context ctx;
>  	struct protocol protocol;
> +	FILE *input = stdin;
>  	int len;
>  	void *buf;
>  	enum {
> @@ -1260,7 +1262,7 @@ int main(int argc, char *argv[])
>  		CODE,
>  	} mode;
>  
> -	if (argc != 2)
> +	if (argc != 2 && argc != 4)
>  		usage(EXIT_FAILURE);
>  	else if (strcmp(argv[1], "help") == 0 || strcmp(argv[1], "--help") == 0)
>  		usage(EXIT_SUCCESS);
> @@ -1273,6 +1275,20 @@ int main(int argc, char *argv[])
>  	else
>  		usage(EXIT_FAILURE);
>  
> +	if (argc == 4) {
> +		input = fopen(argv[2], "r");
> +		if (input == NULL) {
> +			fprintf(stderr, "Could not open input file: %s\n",
> +				strerror(errno));
> +			exit(EXIT_FAILURE);
> +		}
> +		if (freopen(argv[3], "w", stdout) == NULL) {
> +			fprintf(stderr, "Could not open output file: %s\n",
> +				strerror(errno));
> +			exit(EXIT_FAILURE);
> +		}

This part looks good to me now.

> +	}
> +
>  	wl_list_init(&protocol.interface_list);
>  	protocol.type_index = 0;
>  	protocol.null_run_length = 0;
> @@ -1293,7 +1309,7 @@ int main(int argc, char *argv[])
>  
>  	do {
>  		buf = XML_GetBuffer(ctx.parser, XML_BUFFER_SIZE);
> -		len = fread(buf, 1, XML_BUFFER_SIZE, stdin);
> +		len = fread(buf, 1, XML_BUFFER_SIZE, input);
>  		if (len < 0) {
>  			fprintf(stderr, "fread: %m\n");
>  			exit(EXIT_FAILURE);

I think this patch is now technically acceptable.

However, Bill and Bryce had a point. I wonder if we are happy with this
kind of ad-hoc extension of the CLI, after all it is "stable ABI" in a
sense. Would anyone want to do this with proper command line option
parsing, using options instead of adding more "anonymous" position
dependent arguments? Or would it become just even more weird?

Personally I don't really care. There is still room for hacking more
features in if needed, and if we find ourselves in a corner, we could
build another binary with a different name and a new CLI.

Thoughts, anyone?

If no-one objects, I'm inclined to merge this patch with my R-b, say,
next week?


Thanks,
pq