[PATCH weston v5 08/11] drm: Code and comments reformatting for consistency with other backend configs

[Pekka Paalanen]

On Wed, 13 Apr 2016 03:25:12 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  src/compositor-drm.c | 10 ++++++----
>  src/compositor-drm.h | 43 ++++++++++++++++++++++++++++---------------
>  src/main.c           |  6 ++++--
>  3 files changed, 38 insertions(+), 21 deletions(-)
> 

> diff --git a/src/compositor-drm.h b/src/compositor-drm.h
> index 423378a..3b54ca5 100644
> --- a/src/compositor-drm.h
> +++ b/src/compositor-drm.h
> @@ -25,8 +25,8 @@
>   * SOFTWARE.
>   */
>  
> -#ifndef WESTON_COMPOSITOR_DRM_H
> -#define WESTON_COMPOSITOR_DRM_H
> +#ifndef _WESTON_COMPOSITOR_DRM_H
> +#define _WESTON_COMPOSITOR_DRM_H

Hi,

let's not add any more macros starting with an underscore. I suspect
they may be reserved.


> diff --git a/src/main.c b/src/main.c
> index 334464b..f652701 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -675,6 +675,7 @@ load_backend_new(struct weston_compositor *compositor, const char *backend,
>  }
>  
>  
> +// TODO: Why is there a wrapper around the drm config base object?

Looks like a left-over: bad style and removed in a later patch.

>  struct drm_config {
>  	struct weston_drm_backend_config base;
>  	bool use_current_mode;
> @@ -731,7 +732,7 @@ load_drm_backend(struct weston_compositor *c, const char *backend,
>  	struct weston_config_section *section;
>  	int ret = 0;
>  
> -	config = zalloc(sizeof *config);
> +	config = zalloc(sizeof (struct drm_config));

Please keep it as it was to ensure the variable and the sizeof
obviously match. It removes the need for change here in the patch
removing drm_config.

>  	if (!config)
>  		return -1;
>  
> @@ -755,7 +756,8 @@ load_drm_backend(struct weston_compositor *c, const char *backend,
>  	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) {
> +	if (load_backend_new(c, backend,
> +			     (struct weston_backend_config *)(&config->base)) < 0) {

This I don't understand. Why? The old form is shorter, maintains
type-safety, and does not depend on the particular layout of the struct.


>  		ret = -1;
>  		free(config->base.gbm_format);
>  		free(config->base.seat_id);

All the above fixed, this gets:
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160413/94aaf816/attachment.sig>