[vlc-devel] [PATCH 1/2] picture: avoid using void cast

[Steve Lhomme]

I added a static assert on the local structure to make sure we never do 
bogus casts.

On 2019-11-14 11:31, Steve Lhomme wrote:
> Always allocate exactly the type we're going to use, regardless of padding.
> 
> We can allocate and use the extended picture_priv_t locally without having to
> add an extra field in the regular structure.
> ---
>   src/misc/picture.c | 45 ++++++++++++++++++++++++++++++---------------
>   src/misc/picture.h |  2 --
>   2 files changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/src/misc/picture.c b/src/misc/picture.c
> index cdde6f9951a..a49add7355d 100644
> --- a/src/misc/picture.c
> +++ b/src/misc/picture.c
> @@ -193,14 +193,9 @@ int picture_Setup( picture_t *p_picture, const video_format_t *restrict fmt )
>    *
>    *****************************************************************************/
>   
> -static picture_priv_t *picture_NewPrivate(const video_format_t *restrict p_fmt,
> -                                          size_t extra)
> +static bool picture_InitPrivate(const video_format_t *restrict p_fmt,
> +                                picture_priv_t *priv)
>   {
> -    /* */
> -    picture_priv_t *priv = malloc(sizeof (*priv) + extra);
> -    if( unlikely(priv == NULL) )
> -        return NULL;
> -
>       picture_t *p_picture = &priv->picture;
>   
>       memset( p_picture, 0, sizeof( *p_picture ) );
> @@ -210,23 +205,28 @@ static picture_priv_t *picture_NewPrivate(const video_format_t *restrict p_fmt,
>       if( picture_Setup( p_picture, p_fmt ) )
>       {
>           free( p_picture );
> -        return NULL;
> +        return false;
>       }
>   
>       atomic_init(&p_picture->refs, 1);
>       priv->gc.opaque = NULL;
> -
> -    return priv;
> +    return true;
>   }
>   
>   picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_resource_t *p_resource )
>   {
>       assert(p_resource != NULL);
>   
> -    picture_priv_t *priv = picture_NewPrivate(p_fmt, 0);
> -    if (unlikely(priv == NULL))
> +    picture_priv_t *priv = malloc(sizeof(*priv));
> +    if( unlikely(priv == NULL) )
>           return NULL;
>   
> +    if (!picture_InitPrivate(p_fmt, priv))
> +    {
> +        free(priv);
> +        return NULL;
> +    }
> +
>       picture_t *p_picture = &priv->picture;
>   
>       p_picture->p_sys = p_resource->p_sys;
> @@ -248,12 +248,27 @@ picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_r
>   
>   #define PICTURE_SW_SIZE_MAX (UINT32_C(1) << 28) /* 256MB: 8K * 8K * 4*/
>   
> +struct picture_priv_local_t {
> +    picture_priv_t   priv;
> +    picture_buffer_t res;
> +};
> +
>   picture_t *picture_NewFromFormat(const video_format_t *restrict fmt)
>   {
> -    picture_priv_t *priv = picture_NewPrivate(fmt, sizeof (picture_buffer_t));
> -    if (unlikely(priv == NULL))
> +    static_assert(offsetof(struct picture_priv_local_t, priv)==0,
> +                  "misplaced picture_priv_t, destroy won't work");
> +
> +    struct picture_priv_local_t *localpriv = malloc(sizeof(*localpriv));
> +    if( unlikely(localpriv == NULL) )
>           return NULL;
>   
> +    picture_priv_t *priv = &localpriv->priv;
> +    if (!picture_InitPrivate(fmt, priv))
> +    {
> +        free(priv);
> +        return NULL;
> +    }
> +
>       priv->gc.destroy = picture_DestroyFromFormat;
>   
>       picture_t *pic = &priv->picture;
> @@ -278,7 +293,7 @@ picture_t *picture_NewFromFormat(const video_format_t *restrict fmt)
>       if (unlikely(pic_size >= PICTURE_SW_SIZE_MAX))
>           goto error;
>   
> -    picture_buffer_t *res = (void *)priv->extra;
> +    picture_buffer_t *res = &localpriv->res;
>   
>       unsigned char *buf = picture_Allocate(&res->fd, pic_size);
>       if (unlikely(buf == NULL))
> diff --git a/src/misc/picture.h b/src/misc/picture.h
> index 1215214d698..a87164ce866 100644
> --- a/src/misc/picture.h
> +++ b/src/misc/picture.h
> @@ -31,8 +31,6 @@ typedef struct
>           void (*destroy)(picture_t *);
>           void *opaque;
>       } gc;
> -
> -    max_align_t extra[];
>   } picture_priv_t;
>   
>   void *picture_Allocate(int *, size_t);
> -- 
> 2.17.1
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>