[Mesa-dev] [PATCH 6/8 v.2] glcpp: Add --disable-line-continuations argument to standalone glcpp

[Kenneth Graunke]

On 01/03/2013 04:00 PM, Carl Worth wrote:
> This will allow testing of disabled line-continuation on a case-by-case basis,
> (with the option communicated to the preprocessor via the GL context).
> ---
>
> Kenneth Graunke <kenneth at whitecape.org> writes:
>> Why not use getopt()?  The standalone GLSL compiler already does, so it
>> doesn't add another dependency.
>>
>> For the single option you've added here, your code is reasonable, but
>> ad-hoc option parsing gets out of control quick.
>
> Fair enough. See attached updated patch.
>
> Matt Turner <mattst88 at gmail.com> writes:
>> I'd probably just make these six function calls a single one.
>
> This is done as well.
>
> Thanks for the review, guys. I really appreciate it.
>
> -Carl
>
>   src/glsl/glcpp/glcpp.c |   41 +++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
> index 79fbdac..b77dfed 100644
> --- a/src/glsl/glcpp/glcpp.c
> +++ b/src/glsl/glcpp/glcpp.c
> @@ -24,6 +24,8 @@
>   #include <stdio.h>
>   #include <string.h>
>   #include <errno.h>
> +#include <getopt.h>
> +
>   #include "glcpp.h"
>   #include "main/mtypes.h"
>   #include "main/shaderobj.h"
> @@ -100,8 +102,31 @@ static void
>   init_fake_gl_context (struct gl_context *gl_ctx)
>   {
>   	gl_ctx->API = API_OPENGL_COMPAT;
> +	gl_ctx->Const.DisableGLSLLineContinuations = false;
> +}
> +
> +static void
> +usage (void)
> +{
> +	fprintf (stderr,
> +		 "Usage: glcpp [OPTIONS] [--] [<filename>]\n"
> +		 "\n"
> +		 "Pre-process the given filename (stdin if no filename given).\n"
> +		 "The following options are supported:\n"
> +		 "    --disable-line-continuations      Do not interpret lines ending with a\n"
> +		 "                                      backslash ('\\') as a line continuation.\n");
>   }
>
> +enum {
> +	DISABLE_LINE_CONTINUATIONS_OPT = CHAR_MAX + 1
> +};
> +
> +const static struct option
> +long_options[] = {
> +	{"disable-line-continuations", no_argument, 0, DISABLE_LINE_CONTINUATIONS_OPT },
> +	{0,                            0,           0, 0 }
> +};
> +
>   int
>   main (int argc, char *argv[])
>   {
> @@ -111,11 +136,23 @@ main (int argc, char *argv[])
>   	const char *shader;
>   	int ret;
>   	struct gl_context gl_ctx;
> +	int c;
>
>   	init_fake_gl_context (&gl_ctx);
>
> -	if (argc) {
> -		filename = argv[1];
> +	while ((c = getopt_long(argc, argv, "", long_options, NULL)) != -1) {
> +		switch (c) {
> +		case DISABLE_LINE_CONTINUATIONS_OPT:
> +			gl_ctx.Const.DisableGLSLLineContinuations = true;
> +			break;
> +		default:
> +			usage ();
> +			exit (1);
> +		}
> +	}
> +
> +	if (optind < argc) {
> +		filename = argv[optind];
>   	}

How about:

	if (optind == argc - 1) {
		filename = argv[optind];
	} else if (optind < argc) {
		usage ();
		exit (1);
	}

That way, it rejects things like "glcpp foo.frag extracruft".  I tested 
this with various combinations of --disable-line-continuations, --, 
filenames, and extra cruft, and it seems to be correct.

Either way,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

>   	shader = load_text_file (ctx, filename);