[FFmpeg-devel] bprint.h can't be included in C++ code

[wm4]

On Fri, 21 Nov 2014 21:42:35 +0100
Nicolas George <george at nsup.org> wrote:

> Le primidi 1er frimaire, an CCXXIII, Vadim Kalinsky a écrit :
> > bprint.h can't be included in C++ code
> > C++ does not support anonymous struct.
> 
> Thanks for the patch.
> 
> Basically, bprint.h is not meant to be included in c++ code.
> 
> > From a45cc83a807e7eabf158ddff52751171e80874f8 Mon Sep 17 00:00:00 2001
> > From: Vadim Kalinsky <vkalinsky at Q.local>
> > Date: Fri, 21 Nov 2014 13:39:07 -0500
> > Subject: [PATCH] C++ compatible AVBPrint definition.
> > 
> > ---
> >  libavutil/bprint.h | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> > index d1682fc..10e96d7 100644
> > --- a/libavutil/bprint.h
> > +++ b/libavutil/bprint.h
> > @@ -30,9 +30,12 @@
> >   * Define a structure with extra padding to a fixed size
> >   * This helps ensuring binary compatibility with future versions.
> >   */
> > -#define FF_PAD_STRUCTURE(size, ...) \
> > -    __VA_ARGS__ \
> > -    char reserved_padding[size - sizeof(struct { __VA_ARGS__ })];
> 
> > +#define FF_PAD_STRUCTURE(name,size, ...) \
> > +typedef struct __pad_structure_helper_##name { __VA_ARGS__ } __pad_structure_helper_##name; \
> > +typedef struct name { \
> > +__VA_ARGS__ \
> > +char reserved_padding[size - sizeof(__pad_structure_helper_##name)]; \
> > +} name;
> 
> Apart from the fact that it makes the macro hackery vastly less readable
> (maybe some indentation would help), identifiers starting with a double
> underscode are reserved for the implementation, and therefore can not be
> used.

The macro hackery doesn't even have any reason to exist, and even seems
to trigger UB - reserved_internal_buffer is only 1 byte long, and the
code is designed to write past it (and into the reserved member), which
is UB. Flexible array members were invented to rectify and simplify
this, but you could just do without any of this.

> >  
> >  /**
> >   * Buffer to print data progressively
> > @@ -74,15 +77,14 @@
> >   * internal buffer is large enough to hold a reasonable paragraph of text,
> >   * such as the current paragraph.
> >   */
> > -typedef struct AVBPrint {
> > -    FF_PAD_STRUCTURE(1024,
> > +
> > +FF_PAD_STRUCTURE(AVBPrint, 1024,
> >      char *str;         /**< string so far */
> >      unsigned len;      /**< length so far */
> >      unsigned size;     /**< allocated memory */
> >      unsigned size_max; /**< maximum allocated memory */
> >      char reserved_internal_buffer[1];
> > -    )
> > -} AVBPrint;
> > +)
> >  
> >  /**
> >   * Convenience macros for special values for av_bprint_init() size_max
> 
> Regards,
>