[PATCH weston v6 11/12] Enforce destruction of all backend config objects after initialization
Giulio Camuffo
giuliocamuffo at gmail.com
Sun Apr 17 14:22:50 UTC 2016
Maybe it would make sense to squash this too, but either way:
Reviewed-by: Giulio Camuffo <giuliocamuffo at gmail.com>
2016-04-16 6:28 GMT+03:00 Bryce Harrington <bryce at osg.samsung.com>:
> Since the backend config struct versioning implies that there we expect
> potential future descrepancy between main's definition of the config
> object and the backend's, don't allow the backend to hang onto the
> config object outside the initialization scope.
>
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> v6:
> - Put backend configs on stack instead of zalloc()
>
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
> src/main.c | 113 ++++++++++++++++++++++++++++---------------------------------
> 1 file changed, 51 insertions(+), 62 deletions(-)
>
> diff --git a/src/main.c b/src/main.c
> index dcd5ee6..9a8094f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -657,7 +657,20 @@ load_backend_old(struct weston_compositor *compositor, const char *backend,
> return backend_init(compositor, argc, argv, wc, NULL);
> }
>
> -/* Temporary function to be replaced by weston_compositor_load_backend(). */
> +/** Main module call-point for backends.
> + *
> + * All backends should use this routine to access their init routine.
> + * Backends may subclass weston_backend_config to add their own
> + * configuration data, setting the major/minor version in config_base
> + * accordingly.
> + *
> + * The config_base object should be treated as temporary, and any data
> + * copied out of it by backend_init before returning. The load_backend_new
> + * callers may then free the config_base object.
> + *
> + * NOTE: This is a temporary function intended to eventually be replaced
> + * by weston_compositor_load_backend().
> + */
> static int
> load_backend_new(struct weston_compositor *compositor, const char *backend,
> struct weston_backend_config *config_base)
> @@ -678,38 +691,32 @@ static int
> load_drm_backend(struct weston_compositor *c, const char *backend,
> int *argc, char **argv, struct weston_config *wc)
> {
> - struct weston_drm_backend_config *config;
> + struct weston_drm_backend_config config = {{ 0, }};
> struct weston_config_section *section;
> int ret = 0;
>
> - config = zalloc(sizeof (struct weston_drm_backend_config));
> - if (!config)
> - return -1;
> -
> const struct weston_option options[] = {
> - { WESTON_OPTION_INTEGER, "connector", 0, &config->connector },
> - { WESTON_OPTION_STRING, "seat", 0, &config->seat_id },
> - { WESTON_OPTION_INTEGER, "tty", 0, &config->tty },
> - { WESTON_OPTION_BOOLEAN, "current-mode", 0, &config->use_current_mode },
> - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
> + { WESTON_OPTION_INTEGER, "connector", 0, &config.connector },
> + { WESTON_OPTION_STRING, "seat", 0, &config.seat_id },
> + { WESTON_OPTION_INTEGER, "tty", 0, &config.tty },
> + { WESTON_OPTION_BOOLEAN, "current-mode", 0, &config.use_current_mode },
> + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
> };
>
> parse_options(options, ARRAY_LENGTH(options), argc, argv);
>
> section = weston_config_get_section(wc, "core", NULL, NULL);
> weston_config_section_get_string(section,
> - "gbm-format", &config->gbm_format,
> + "gbm-format", &config.gbm_format,
> NULL);
>
> - config->base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
> - config->base.struct_size = sizeof(struct weston_drm_backend_config);
> + config.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
> + config.base.struct_size = sizeof(struct weston_drm_backend_config);
>
> - ret = load_backend_new(c, backend,
> - (struct weston_backend_config *)config);
> + ret = load_backend_new(c, backend, &config.base);
>
> - free(config->gbm_format);
> - free(config->seat_id);
> - free(config);
> + free(config.gbm_format);
> + free(config.seat_id);
>
> return ret;
> }
> @@ -718,38 +725,30 @@ static int
> load_headless_backend(struct weston_compositor *c, char const * backend,
> int *argc, char **argv, struct weston_config *wc)
> {
> - struct weston_headless_backend_config *config;
> + struct weston_headless_backend_config config = {{ 0, }};
> int ret = 0;
> const char *transform = "normal";
>
> - config = zalloc(sizeof(struct weston_headless_backend_config));
> - if (config == NULL)
> - return -1;
> -
> - config->width = 1024;
> - config->height = 640;
> + config.width = 1024;
> + config.height = 640;
>
> const struct weston_option options[] = {
> - { WESTON_OPTION_INTEGER, "width", 0, &config->width },
> - { WESTON_OPTION_INTEGER, "height", 0, &config->height },
> - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
> + { WESTON_OPTION_INTEGER, "width", 0, &config.width },
> + { WESTON_OPTION_INTEGER, "height", 0, &config.height },
> + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
> { WESTON_OPTION_STRING, "transform", 0, &transform },
> };
>
> parse_options(options, ARRAY_LENGTH(options), argc, argv);
>
> - if (weston_parse_transform(transform, &config->transform) < 0)
> + if (weston_parse_transform(transform, &config.transform) < 0)
> weston_log("Invalid transform \"%s\"\n", transform);
>
> - config->base.struct_version = WESTON_HEADLESS_BACKEND_CONFIG_VERSION;
> - config->base.struct_size = sizeof(struct weston_headless_backend_config);
> + config.base.struct_version = WESTON_HEADLESS_BACKEND_CONFIG_VERSION;
> + config.base.struct_size = sizeof(struct weston_headless_backend_config);
>
> /* load the actual wayland backend and configure it */
> - if (load_backend_new(c, backend,
> - (struct weston_backend_config *)config) < 0) {
> - ret = -1;
> - free(config);
> - }
> + ret = load_backend_new(c, backend, &config.base);
>
> return ret;
> }
> @@ -774,7 +773,7 @@ load_x11_backend(struct weston_compositor *c, char const * backend,
> int *argc, char **argv, struct weston_config *wc)
> {
> struct weston_x11_backend_output_config default_output;
> - struct weston_x11_backend_config *config;
> + struct weston_x11_backend_config config = {{ 0, }};
> struct weston_config_section *section;
> int ret = 0;
> int option_width = 0;
> @@ -786,18 +785,14 @@ load_x11_backend(struct weston_compositor *c, char const * backend,
> int i;
> uint32_t j;
>
> - config = zalloc(sizeof(struct weston_x11_backend_config));
> - if (config == NULL)
> - return -1;
> -
> const struct weston_option options[] = {
> { WESTON_OPTION_INTEGER, "width", 0, &option_width },
> { WESTON_OPTION_INTEGER, "height", 0, &option_height },
> { WESTON_OPTION_INTEGER, "scale", 0, &option_scale },
> - { WESTON_OPTION_BOOLEAN, "fullscreen", 'f', &config->fullscreen },
> + { WESTON_OPTION_BOOLEAN, "fullscreen", 'f', &config.fullscreen },
> { WESTON_OPTION_INTEGER, "output-count", 0, &option_count },
> - { WESTON_OPTION_BOOLEAN, "no-input", 0, &config->no_input },
> - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
> + { WESTON_OPTION_BOOLEAN, "no-input", 0, &config.no_input },
> + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
> };
>
> parse_options(options, ARRAY_LENGTH(options), argc, argv);
> @@ -847,9 +842,9 @@ load_x11_backend(struct weston_compositor *c, char const * backend,
> t, current_output.name);
> free(t);
>
> - if (weston_x11_backend_config_append_output_config(config, ¤t_output) < 0) {
> + if (weston_x11_backend_config_append_output_config(&config, ¤t_output) < 0) {
> ret = -1;
> - goto error;
> + goto out;
> }
>
> output_count++;
> @@ -869,29 +864,23 @@ load_x11_backend(struct weston_compositor *c, char const * backend,
> goto error;
> }
>
> - if (weston_x11_backend_config_append_output_config(config, &default_output) < 0) {
> + if (weston_x11_backend_config_append_output_config(&config, &default_output) < 0) {
> ret = -1;
> - goto error;
> + goto out;
> }
> }
>
> - config->base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
> - config->base.struct_size = sizeof(struct weston_x11_backend_config);
> + config.base.struct_version = WESTON_X11_BACKEND_CONFIG_VERSION;
> + config.base.struct_size = sizeof(struct weston_x11_backend_config);
>
> /* load the actual drm backend and configure it */
> - if (load_backend_new(c, backend,
> - (struct weston_backend_config *)config) < 0) {
> - ret = -1;
> - goto error;
> - }
> + ret = load_backend_new(c, backend, &config.base);
>
> - return ret;
> +out:
> + for (j = 0; j < config.num_outputs; ++j)
> + free(config.outputs[j].name);
> + free(config.outputs);
>
> -error:
> - for (j = 0; j < config->num_outputs; ++j)
> - free(config->outputs[j].name);
> - free(config->outputs);
> - free(config);
> return ret;
> }
>
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list