Re: [PATCH weston v2 7/9] wayland-backend: split init_backend

[Pekka Paalanen]

On Thu, 28 Apr 2016 20:33:14 +0200
Benoit Gschwind  wrote:

> Extract configuration parsing from init_backend function. The new
> init_backend start by generating the configuration structure from
> configuration file with separated function and then use this structure
> to load the backend.
> 
> Signed-off-by: Benoit Gschwind 
> ---
>  src/compositor-wayland.c | 179 
> +--
>  1 file changed, 127 insertions(+), 52 deletions(-)
> 
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index e40403f..58593bd 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -67,6 +67,8 @@ struct weston_wayland_backend_config {
>   int fullscreen;
>   char *cursor_theme;
>   int cursor_size;
> + int num_outputs;
> + struct weston_wayland_backend_output_config *outputs;
>  };
>  
>  struct wayland_backend {
> @@ -2341,18 +2343,43 @@ wayland_backend_destroy(struct wayland_backend *b)
>   free(b);
>  }
>  
> -WL_EXPORT int
> -backend_init(struct weston_compositor *compositor, int *argc, char *argv[],
> +static void
> +wayland_backend_config_release(struct weston_wayland_backend_config 
> *new_config) {
> + int i;

One empty line between declarations and code.

> + for (i = 0; i < new_config->num_outputs; ++i) {
> + free(new_config->outputs[i].name);
> + }
> + free(new_config->cursor_theme);
> + free(new_config->display_name);
> + free(new_config->outputs);
> +}
> +
> +/*
> + * Append a new output struct at the end of new_config.outputs and return a
> + * pointer to the newly allocated structure or NULL if fail. The allocated
> + * structure is NOT cleared nor set to default values.
> + */
> +static struct weston_wayland_backend_output_config *
> +wayland_backend_config_add_new_output(struct weston_wayland_backend_config 
> *new_config) {
> + struct weston_wayland_backend_output_config *outputs;
> + outputs = realloc(new_config->outputs,
> +   (new_config->num_outputs+1)*sizeof(struct 
> weston_wayland_backend_output_config));

All binary operators must have spaces on both sides.

> + if (!outputs)
> + return NULL;
> + new_config->num_outputs += 1;
> + new_config->outputs = outputs;
> + return &(new_config->outputs[new_config->num_outputs-1]);

All binary operators must have spaces on both sides.

> +}
> +

A whitespace fix-up patch would be welcome.


Thanks,
pq


pgpVC_3QhwVRL.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 7/9] wayland-backend: split init_backend

[Benoit Gschwind]

Hello Yong,

Thanks, I will fix this in next version after few more review :)

Best regards.

Le 29/04/2016 03:38, Yong Bakos a écrit :
> On Apr 28, 2016, at 1:33 PM, Benoit Gschwind  wrote:
>>
>> Extract configuration parsing from init_backend function. The new
>> init_backend start by generating the configuration structure from
>> configuration file with separated function and then use this structure
>> to load the backend.
>>
>> Signed-off-by: Benoit Gschwind 
> 
> Hi Benoit, one minor whitespace issue, noted below.
> 
> yong
> 
> 
>> ---
>> src/compositor-wayland.c | 179 
>> +--
>> 1 file changed, 127 insertions(+), 52 deletions(-)
>>
>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>> index e40403f..58593bd 100644
>> --- a/src/compositor-wayland.c
>> +++ b/src/compositor-wayland.c
>> @@ -67,6 +67,8 @@ struct weston_wayland_backend_config {
>>  int fullscreen;
>>  char *cursor_theme;
>>  int cursor_size;
>> +int num_outputs;
>> +struct weston_wayland_backend_output_config *outputs;
>> };
>>
>> struct wayland_backend {
>> @@ -2341,18 +2343,43 @@ wayland_backend_destroy(struct wayland_backend *b)
>>  free(b);
>> }
>>
>> -WL_EXPORT int
>> -backend_init(struct weston_compositor *compositor, int *argc, char *argv[],
>> +static void
>> +wayland_backend_config_release(struct weston_wayland_backend_config 
>> *new_config) {
>> +int i;
>> +for (i = 0; i < new_config->num_outputs; ++i) {
>> +free(new_config->outputs[i].name);
>> +}
>> +free(new_config->cursor_theme);
>> +free(new_config->display_name);
>> +free(new_config->outputs);
>> +}
>> +
>> +/*
>> + * Append a new output struct at the end of new_config.outputs and return a
>> + * pointer to the newly allocated structure or NULL if fail. The allocated
>> + * structure is NOT cleared nor set to default values.
>> + */
>> +static struct weston_wayland_backend_output_config *
>> +wayland_backend_config_add_new_output(struct weston_wayland_backend_config 
>> *new_config) {
>> +struct weston_wayland_backend_output_config *outputs;
>> +outputs = realloc(new_config->outputs,
>> +  (new_config->num_outputs+1)*sizeof(struct 
>> weston_wayland_backend_output_config));
>> +if (!outputs)
>> +return NULL;
>> +new_config->num_outputs += 1;
>> +new_config->outputs = outputs;
>> +return &(new_config->outputs[new_config->num_outputs-1]);
>> +}
>> +
>> +static int
>> +load_wayland_backend_config(struct weston_compositor *compositor, int 
>> *argc, char *argv[],
>>   struct weston_config *config,
>> - struct weston_backend_config *config_base)
>> + struct weston_wayland_backend_config *out_config)
>> {
>> -struct wayland_backend *b;
>> -struct wayland_output *output;
>> -struct wayland_parent_output *poutput;
>> +struct weston_wayland_backend_config new_config = { 0, };
>>  struct weston_config_section *section;
>> -struct weston_wayland_backend_config new_config;
>> -struct weston_wayland_backend_output_config output_config;
>> -int x, count, width, height, scale;
>> +struct weston_wayland_backend_output_config *oc;
>> +int count, width, height, scale;
>>  const char *section_name;
>>  char *name;
>>
>> @@ -2387,39 +2414,30 @@ backend_init(struct weston_compositor *compositor, 
>> int *argc, char *argv[],
>>  weston_config_section_get_int(section, "cursor-size",
>>_config.cursor_size, 32);
>>
>> -b = wayland_backend_create(compositor, _config, argc, argv, config);
>> -
>> -if (!b)
>> -return -1;
>> -
>> -if (new_config.sprawl || b->parent.fshell) {
>> -b->sprawl_across_outputs = 1;
>> -wl_display_roundtrip(b->parent.wl_display);
>> -
>> -wl_list_for_each(poutput, >parent.output_list, link)
>> -wayland_output_create_for_parent_output(b, poutput);
>> -
>> +if (new_config.sprawl) {
>> +/* do nothing, everything is already set */
>> +*out_config = new_config;
>>  return 0;
>>  }
>>
>>  if (new_config.fullscreen) {
>> -output_config.width = width;
>> -output_config.height = height;
>> -output_config.name = NULL;
>> -output_config.transform = WL_OUTPUT_TRANSFORM_NORMAL;
>> -output_config.scale = scale;
>>
>> -output = wayland_output_create_for_config(b, _config,
>> -  1, 0, 0);
>> -if (!output)
>> +oc = wayland_backend_config_add_new_output(_config);
>> +
>> +if (!oc)
>>  goto err_outputs;
>>
>> -wayland_output_set_fullscreen(output, 0, 0, NULL);
>> +oc->width = width;
>> +oc->height = height;
>> +

Re: [PATCH weston v2 7/9] wayland-backend: split init_backend

[Yong Bakos]

On Apr 28, 2016, at 1:33 PM, Benoit Gschwind  wrote:
> 
> Extract configuration parsing from init_backend function. The new
> init_backend start by generating the configuration structure from
> configuration file with separated function and then use this structure
> to load the backend.
> 
> Signed-off-by: Benoit Gschwind 

Hi Benoit, one minor whitespace issue, noted below.

yong


> ---
> src/compositor-wayland.c | 179 +--
> 1 file changed, 127 insertions(+), 52 deletions(-)
> 
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index e40403f..58593bd 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -67,6 +67,8 @@ struct weston_wayland_backend_config {
>   int fullscreen;
>   char *cursor_theme;
>   int cursor_size;
> + int num_outputs;
> + struct weston_wayland_backend_output_config *outputs;
> };
> 
> struct wayland_backend {
> @@ -2341,18 +2343,43 @@ wayland_backend_destroy(struct wayland_backend *b)
>   free(b);
> }
> 
> -WL_EXPORT int
> -backend_init(struct weston_compositor *compositor, int *argc, char *argv[],
> +static void
> +wayland_backend_config_release(struct weston_wayland_backend_config 
> *new_config) {
> + int i;
> + for (i = 0; i < new_config->num_outputs; ++i) {
> + free(new_config->outputs[i].name);
> + }
> + free(new_config->cursor_theme);
> + free(new_config->display_name);
> + free(new_config->outputs);
> +}
> +
> +/*
> + * Append a new output struct at the end of new_config.outputs and return a
> + * pointer to the newly allocated structure or NULL if fail. The allocated
> + * structure is NOT cleared nor set to default values.
> + */
> +static struct weston_wayland_backend_output_config *
> +wayland_backend_config_add_new_output(struct weston_wayland_backend_config 
> *new_config) {
> + struct weston_wayland_backend_output_config *outputs;
> + outputs = realloc(new_config->outputs,
> +   (new_config->num_outputs+1)*sizeof(struct 
> weston_wayland_backend_output_config));
> + if (!outputs)
> + return NULL;
> + new_config->num_outputs += 1;
> + new_config->outputs = outputs;
> + return &(new_config->outputs[new_config->num_outputs-1]);
> +}
> +
> +static int
> +load_wayland_backend_config(struct weston_compositor *compositor, int *argc, 
> char *argv[],
>struct weston_config *config,
> -  struct weston_backend_config *config_base)
> +  struct weston_wayland_backend_config *out_config)
> {
> - struct wayland_backend *b;
> - struct wayland_output *output;
> - struct wayland_parent_output *poutput;
> + struct weston_wayland_backend_config new_config = { 0, };
>   struct weston_config_section *section;
> - struct weston_wayland_backend_config new_config;
> - struct weston_wayland_backend_output_config output_config;
> - int x, count, width, height, scale;
> + struct weston_wayland_backend_output_config *oc;
> + int count, width, height, scale;
>   const char *section_name;
>   char *name;
> 
> @@ -2387,39 +2414,30 @@ backend_init(struct weston_compositor *compositor, 
> int *argc, char *argv[],
>   weston_config_section_get_int(section, "cursor-size",
> _config.cursor_size, 32);
> 
> - b = wayland_backend_create(compositor, _config, argc, argv, config);
> -
> - if (!b)
> - return -1;
> -
> - if (new_config.sprawl || b->parent.fshell) {
> - b->sprawl_across_outputs = 1;
> - wl_display_roundtrip(b->parent.wl_display);
> -
> - wl_list_for_each(poutput, >parent.output_list, link)
> - wayland_output_create_for_parent_output(b, poutput);
> -
> + if (new_config.sprawl) {
> + /* do nothing, everything is already set */
> + *out_config = new_config;
>   return 0;
>   }
> 
>   if (new_config.fullscreen) {
> - output_config.width = width;
> - output_config.height = height;
> - output_config.name = NULL;
> - output_config.transform = WL_OUTPUT_TRANSFORM_NORMAL;
> - output_config.scale = scale;
> 
> - output = wayland_output_create_for_config(b, _config,
> -   1, 0, 0);
> - if (!output)
> + oc = wayland_backend_config_add_new_output(_config);
> +
> + if (!oc)
>   goto err_outputs;
> 
> - wayland_output_set_fullscreen(output, 0, 0, NULL);
> + oc->width = width;
> + oc->height = height;
> + oc->name = NULL;
> + oc->transform = WL_OUTPUT_TRANSFORM_NORMAL;
> + oc->scale = 1;
> +
> + *out_config = new_config;
>   return 0;
>   }
> 
>