Hello,
I think that struct_version and struct_size should not be belong
compositor.h. I think those versioning should be back-end detail, and
each back-end should provide a major version number through #define in
the backend header.
Otherwise the patch look good
Best regards.
Reviewed-by: Benoit Gschwind <gschw...@gnu-log.net>
Le 10/03/2016 01:49, Bryce Harrington a écrit :
> With this struct versioning, it is possible to add new options without
> breaking the ABI, as long as all additions are made to the end of a
> struct and nothing existing is modified or removed. When things are
> added, the structure's size will increase, and we'll use this size as
> our minor version number. If existing things need to be changed, then
> the major version, struct_version, is incremented to indicate the ABI
> break.
>
> From our call site in main we record these major and minor version as
> struct_version and struct_size. The backend then verifies these against
> its own assumptions. So long as the backend's struct is equal or larger
> than what was passed in and the major versions are equal, we're good;
> but if it is larger, then this is a fatal error.
>
> Signed-off-by: Bryce Harrington <br...@osg.samsung.com>
> ---
> src/compositor-drm.c | 10 ++++++++--
> src/compositor.h | 16 ++++++++++++++++
> src/main.c | 2 ++
> 3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 5ddedb9..9bce285 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -3203,8 +3203,14 @@ backend_init(struct weston_compositor *compositor, int
> *argc, char *argv[],
> struct weston_backend_config *config_base)
> {
> struct drm_backend *b;
> - struct weston_drm_backend_config *config =
> - (struct weston_drm_backend_config *)config_base;
> + struct weston_drm_backend_config *config;
> +
> + if (config_base == NULL ||
> + config_base->struct_version != 1 ||
> + config_base->struct_size > sizeof(struct weston_drm_backend_config))
> + return -1;
> +
> + config = (struct weston_drm_backend_config *)config_base;
>
> b = drm_backend_create(compositor, config);
> if (b == NULL)
> diff --git a/src/compositor.h b/src/compositor.h
> index 30462cf..3e52703 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -684,6 +684,22 @@ struct weston_backend_output_config {
> * data.
> */
> struct weston_backend_config {
> + /** Major version for the backend-specific config struct
> + *
> + * This version must match exactly what the backend expects, otherwise
> + * the struct is incompatible.
> + */
> + uint32_t struct_version;
> +
> + /** Minor version of the backend-specific config struct
> + *
> + * This must be set to sizeof(struct backend-specific config).
> + * If the value here is smaller than what the backend expects, the
> + * extra config members will assume their default values.
> + *
> + * A value greater than what the backend expects is incompatible.
> + */
> + size_t struct_size;
> };
>
> struct weston_backend {
> diff --git a/src/main.c b/src/main.c
> index 66c054e..7370292 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -748,6 +748,8 @@ load_drm_backend(struct weston_compositor *c, const char
> *backend,
> "gbm-format", &config->base.format,
> NULL);
>
> + config->base.base.struct_version = 1;
> + config->base.base.struct_size = sizeof(struct
> weston_drm_backend_config);
> config->base.configure_output = drm_configure_output;
>
> if (load_backend_new(c, backend, &config->base.base) < 0) {
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
