[PATCH 4/5] compositor: Version the backend configuration structures

[Benoit Gschwind]

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 <gschwind at 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 <bryce at 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) {
>