[PATCH weston 4/6] shared/option-parser: Add string list options

[Pekka Paalanen]

On Tue, 23 Apr 2013 14:54:48 +0200
Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:

> From: Quentin Glidic <sardemff7+git at sardemff7.net>
> 
> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> ---
>  man/weston.man         |  6 +++---
>  shared/config-parser.h |  1 +
>  shared/option-parser.c | 13 +++++++++++++
>  src/compositor.c       | 27 ++++++++++++++++++++++++---
>  tests/weston-tests-env |  6 ++++--
>  5 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/man/weston.man b/man/weston.man
> index 97db3c8..1812be7 100644
> --- a/man/weston.man
> +++ b/man/weston.man
> @@ -127,9 +127,9 @@ Append log messages to the file
>  .I file.log
>  instead of writing them to stderr.
>  .TP
> -\fB\-\-modules\fR=\fImodule1.so,module2.so\fR
> -Load the comma-separated list of modules. Only used by the test
> -suite. The file is searched for in
> +\fB\-\-modules\fR=\fImodule.so\fR
> +Load the modules (the option may be specified multiple times).
> +Only used by the test suite. The file is searched for in
>  .IR "__weston_modules_dir__" ,
>  or you can pass a path.
>  .TP
> diff --git a/shared/config-parser.h b/shared/config-parser.h
> index 1d0ee3f..37af064 100644
> --- a/shared/config-parser.h
> +++ b/shared/config-parser.h
> @@ -59,6 +59,7 @@ enum weston_option_type {
>  	WESTON_OPTION_INTEGER,
>  	WESTON_OPTION_UNSIGNED_INTEGER,
>  	WESTON_OPTION_STRING,
> +	WESTON_OPTION_STRING_LIST,
>  	WESTON_OPTION_BOOLEAN
>  };
>  
> diff --git a/shared/option-parser.c b/shared/option-parser.c
> index 023fe72..9e0a740 100644
> --- a/shared/option-parser.c
> +++ b/shared/option-parser.c
> @@ -32,6 +32,8 @@
>  static bool
>  handle_option(const struct weston_option *option, char *value)
>  {
> +	char **string_list, **str;
> +	int size = 0;
>  	switch (option->type) {
>  	case WESTON_OPTION_INTEGER:
>  		* (int32_t *) option->data = strtol(value, NULL, 0);
> @@ -42,6 +44,17 @@ handle_option(const struct weston_option *option, char *value)
>  	case WESTON_OPTION_STRING:
>  		* (char **) option->data = strdup(value);
>  		return true;
> +	case WESTON_OPTION_STRING_LIST:
> +		string_list = * (char ***) option->data;
> +		if (string_list != NULL) {
> +			for (str = string_list; *str != NULL; ++str)
> +				++size;
> +		}
> +		string_list = realloc(string_list, (size+2) * sizeof(char *));
> +		string_list[size] = strdup(value);
> +		string_list[size+1] = NULL;
> +		* (char ***) option->data = string_list;
> +		return true;
>  	case WESTON_OPTION_BOOLEAN:
>  		* (int32_t *) option->data = 1;
>  		return false;
> diff --git a/src/compositor.c b/src/compositor.c
> index fe51061..5296df2 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3406,6 +3406,26 @@ load_modules(struct weston_compositor *ec, const char *modules,
>  	return 0;
>  }
>  
> +static int
> +load_modules_strv(struct weston_compositor *ec, char **modules,
> +		  int *argc, char *argv[], const char *config_file)
> +{
> +	char **module;
> +	int (*module_init)(struct weston_compositor *ec,
> +			   int *argc, char *argv[], const char *config_file);
> +
> +	if (modules == NULL)
> +		return 0;
> +
> +	for (module = modules; *module != NULL; ++module) {
> +		module_init = load_module(*module, "module_init");
> +		if (module_init)
> +			module_init(ec, argc, argv, config_file);
> +	}
> +
> +	return 0;
> +}
> +
>  static const char xdg_error_message[] =
>  	"fatal: environment variable XDG_RUNTIME_DIR is not set.\n";
>  
> @@ -3525,7 +3545,8 @@ int main(int argc, char *argv[])
>  				 int *argc, char *argv[], const char *config_file);
>  	int i;
>  	char *backend = NULL;
> -	const char *modules = "desktop-shell.so", *option_modules = NULL;
> +	const char *modules = "desktop-shell.so";
> +	char **option_modules = NULL;
>  	char *log = NULL;
>  	int32_t idle_time = 300;
>  	int32_t help = 0;
> @@ -3546,7 +3567,7 @@ int main(int argc, char *argv[])
>  		{ WESTON_OPTION_STRING, "backend", 'B', &backend },
>  		{ WESTON_OPTION_STRING, "socket", 'S', &socket_name },
>  		{ WESTON_OPTION_INTEGER, "idle-time", 'i', &idle_time },
> -		{ WESTON_OPTION_STRING, "modules", 0, &option_modules },
> +		{ WESTON_OPTION_STRING_LIST, "modules", 0, &option_modules },
>  		{ WESTON_OPTION_STRING, "log", 0, &log },
>  		{ WESTON_OPTION_BOOLEAN, "help", 'h', &help },
>  		{ WESTON_OPTION_BOOLEAN, "version", 0, &version },
> @@ -3619,7 +3640,7 @@ int main(int argc, char *argv[])
>  
>  	if (load_modules(ec, modules, &argc, argv, config_file) < 0)
>  		goto out;
> -	if (load_modules(ec, option_modules, &argc, argv, config_file) < 0)
> +	if (load_modules_strv(ec, option_modules, &argc, argv, config_file) < 0)
>  		goto out;
>  
>  	free(config_file);

There's a free(option_modules) further down below, right?
What about freeing all the individual strings stored in it?

> diff --git a/tests/weston-tests-env b/tests/weston-tests-env
> index 8ae0bcf..ed10d68 100755
> --- a/tests/weston-tests-env
> +++ b/tests/weston-tests-env
> @@ -22,7 +22,8 @@ case $1 in
>  	*.la|*.so)
>  		$WESTON --backend=$BACKEND \
>  			--socket=test-$(basename $1) \
> -			--modules=$abs_builddir/.libs/${1/.la/.so},xwayland.so \
> +			--modules=xwayland.so \
> +			--modules=$abs_builddir/.libs/${1/.la/.so} \
>  			--log="$SERVERLOG" \
>  			&> "$OUTLOG"
>  		;;
> @@ -31,7 +32,8 @@ case $1 in
>  			--socket=test-$(basename $1) \
>  			--backend=$BACKEND \
>  			--log="$SERVERLOG" \
> -			--modules=$abs_builddir/.libs/weston-test.so,xwayland.so \
> +			--modules=xwayland.so \
> +			--modules=$abs_builddir/.libs/weston-test.so \
>  			$abs_builddir/$1 \
>  			&> "$OUTLOG"
>  esac

Otherwise seems ok, though this does change the format of the --modules
option.


Thanks,
pq