[PATCH 3/3] compositor: dump command line options

[Tiago Vignatti]

On 03/19/2012 09:08 AM, Pekka Paalanen wrote:
> On Mon, 12 Mar 2012 19:06:40 -0300
> Tiago Vignatti<tiago.vignatti at intel.com>  wrote:
>
>> Use it with --help or -h.
>>
>> Signed-off-by: Tiago Vignatti<tiago.vignatti at intel.com>
>> ---
>>   shared/config-parser.h   |    3 +++
>>   shared/option-parser.c   |   13 +++++++++++++
>>   src/compositor-drm.c     |    7 ++++++-
>>   src/compositor-openwfd.c |    7 ++++++-
>>   src/compositor-wayland.c |    8 +++++++-
>>   src/compositor-x11.c     |    8 +++++++-
>>   src/compositor.c         |    8 ++++++++
>>   7 files changed, 50 insertions(+), 4 deletions(-)
>>
>> diff --git a/shared/config-parser.h b/shared/config-parser.h
>> index 6403947..4792bcb 100644
>> --- a/shared/config-parser.h
>> +++ b/shared/config-parser.h
>> @@ -67,6 +67,9 @@ struct weston_option {
>>   	void *data;
>>   };
>>
>> +void
>> +dump_options(const struct weston_option *option, int count);
>> +
>>   int
>>   parse_options(const struct weston_option *options,
>>   	      int count, int argc, char *argv[]);
>> diff --git a/shared/option-parser.c b/shared/option-parser.c
>> index 8b16b5d..bc977c1 100644
>> --- a/shared/option-parser.c
>> +++ b/shared/option-parser.c
>> @@ -70,6 +70,19 @@ check_uint(const char *value)
>>   	}
>>   }
>>
>> +void
>> +dump_options(const struct weston_option *option, int count)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i<  count; i++) {
>> +		fprintf(stderr, "--%s", option[i].name);
>> +		if (option[i].short_name)
>> +			fprintf(stderr, ", -%c", option[i].short_name);
>> +		fprintf(stderr, "\n", option[i].name);
>> +	}
>> +}
>> +
>>   static void
>>   handle_option(const struct weston_option *option, char *value)
>>   {
> ...
>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>> index b6b28de..baa1f40 100644
>> --- a/src/compositor-x11.c
>> +++ b/src/compositor-x11.c
>> @@ -824,16 +824,22 @@ x11_compositor_create(struct wl_display *display,
>>   WL_EXPORT struct weston_compositor *
>>   backend_init(struct wl_display *display, int argc, char *argv[])
>>   {
>> -	int width = 1024, height = 640, fullscreen = 0, count = 1;
>> +	int width = 1024, height = 640, fullscreen = 0, count = 1, help = 0;
>>
>>   	const struct weston_option x11_options[] = {
>>   		{ WESTON_OPTION_UNSIGNED_INTEGER, "width", 0,&width },
>>   		{ WESTON_OPTION_UNSIGNED_INTEGER, "height", 0,&height },
>>   		{ WESTON_OPTION_BOOLEAN, "fullscreen", 0,&fullscreen },
>>   		{ WESTON_OPTION_UNSIGNED_INTEGER, "output-count", 0,&count },
>> +		{ WESTON_OPTION_BOOLEAN, "help", 'h',&help },
>>   	};
>>
>>   	parse_options(x11_options, ARRAY_LENGTH(x11_options), argc, argv);
>> +	if (help) {
>> +		dump_options(x11_options, ARRAY_LENGTH(x11_options) - 1);
>> +		exit (EXIT_SUCCESS);
>> +	}
>> +
>>
>>   	return x11_compositor_create(display,
>>   				     width, height, count, fullscreen);
>> diff --git a/src/compositor.c b/src/compositor.c
>> index a832d3f..c50fa3d 100644
>> --- a/src/compositor.c
>> +++ b/src/compositor.c
>> @@ -2467,6 +2467,7 @@ int main(int argc, char *argv[])
>>   	char *shell = NULL;
>>   	int32_t idle_time = 300;
>>   	int32_t xserver;
>> +	int32_t help = 0;
>>   	char *socket_name = NULL;
>>
>>   	const struct weston_option core_options[] = {
>> @@ -2475,6 +2476,7 @@ int main(int argc, char *argv[])
>>   		{ WESTON_OPTION_UNSIGNED_INTEGER, "idle-time", 'i',&idle_time },
>>   		{ WESTON_OPTION_STRING, "shell", 's',&shell },
>>   		{ WESTON_OPTION_BOOLEAN, "xserver", 0,&xserver },
>> +		{ WESTON_OPTION_BOOLEAN, "help", 'h',&help },
>>   	};
>>
>>   	argc = parse_options(core_options,
>> @@ -2499,6 +2501,12 @@ int main(int argc, char *argv[])
>>   	sigemptyset(&segv_action.sa_mask);
>>   	sigaction(SIGSEGV,&segv_action, NULL);
>>
>> +	if (help) {
>> +		dump_options(core_options, ARRAY_LENGTH(core_options));
>> +		argv[argc] = strdup("--help");
>> +		argc++;
>
> How do you know there is space in argv[] to add that?
> Also, doesn't strdup() leak here?
>
> Hm, so if there was a --help originally, there must be space to add it
> back... I wonder if it made sense to handle --help specially in option
> parser, and let it indicate "help encountered" with an error code
> return. We are going to need the error code return anyway.

you're right. So argv is at stack and can be referenceable, so maybe we 
can use the following to get the address back:

+       if (help) {
+               dump_options(core_options, ARRAY_LENGTH(core_options));
+               argv[argc] = argv[argc - 1] + strlen(argv[argc - 1]) +1;
+               strcpy(argv[argc], "--help");
+               argc++;
+       }

is there a cleaner way maybe to get?

And I didn't want to prolong too much with this all, for instance 
treating --help as special case; not sure we'll soon need an error code 
return there, as you mentioned. Why you say so? I'm seeing the patch as 
it is as an improvement already and, frankly, it's quite lean.


Thank you,

    Tiago