[PATCH] Override modules list and don't always load desktop-shell.so

Emilio Pozuelo Monfort pochu27 at gmail.com
Mon Apr 1 10:02:53 PDT 2013


Hi,

The patch looks good to me, but it should come with an update of the --help
output, which currently states:

--modules		Load the comma-separated list of modules

This should state that it will override any modules listed in weston.ini.

And the manpage says:

--modules=module1.so,module2.so
       Load  the comma-separated list of modules. Only used by the test suite.
       The file is searched for in /opt/wayland/lib/weston, or you can pass an
       absolute path.

Likewise here, plus I think the statement about the test suite is obsolete and
should be removed.

Also I have one little suggestion:

On 04/01/2013 04:31 PM, Pier Luigi Fiorini wrote:
> Let --modules override modules list and load desktop-shell.so as a
> fallback if a modules list is not specified neither by passing
> --modules nor with weston.ini.
> ---
>  src/compositor.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 1617d96..e2475c8 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -3611,10 +3611,13 @@ int main(int argc, char *argv[])
>  
>  	setenv("WAYLAND_DISPLAY", socket_name, 1);
>  
> -	if (load_modules(ec, modules, &argc, argv, config_file) < 0)
> -		goto out;
> -	if (load_modules(ec, option_modules, &argc, argv, config_file) < 0)
> -		goto out;
> +	if (option_modules == NULL) {
> +		if (load_modules(ec, modules, &argc, argv, config_file) < 0)
> +			goto out;
> +	} else {
> +		if (load_modules(ec, option_modules, &argc, argv, config_file) < 0)
> +			goto out;
> +	}

I would rather do something like this to simplify the code (at least it looks
clearer to me but YMMV):

	if (load_modules(ec, option_modules != NULL ? option_modules : modules,
            &argc, argv, config_file) < 0)
		goto out;

>  
>  	free(config_file);
>  
> 

Cheers,
Emilio


More information about the wayland-devel mailing list