Maybe it would make sense to squash this too, but either way: Reviewed-by: Giulio Camuffo <[email protected]>
2016-04-16 6:28 GMT+03:00 Bryce Harrington <[email protected]>: > 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 <[email protected]> > Reviewed-by: Pekka Paalanen <[email protected]> > > v6: > - Put backend configs on stack instead of zalloc() > > Signed-off-by: Bryce Harrington <[email protected]> > --- > 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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
