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

[Nicolas George]

Le tridi 3 frimaire, an CCXXIII, Vadim Kalinsky a écrit :
> Yep, my bad.

Thanks for the updated patch.

> Maybe I'm little contaminated with modern PLs, but I don't see huge
> difference in readability between the old and the new versions. Both are
> terrible :-)

At least, this one, I can actually read what is being changed directly in
the patch.

> From 6e97656e9765b734ca66d1665eba190e37be3312 Mon Sep 17 00:00:00 2001
> From: Vadim Kalinsky <vkalinsky at Q.local>
> Date: Sun, 23 Nov 2014 21:29:25 -0500
> Subject: [PATCH] Squashed
> 
> ---
>  libavutil/bprint.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> index d1682fc..4df57ce 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, ...) \

> +#define FF_PAD_STRUCTURE(name,size, ...) \
                                 ^
Nit: space after commas.

> +typedef struct pad_structure_helper_##name { __VA_ARGS__ } pad_structure_helper_##name; \

Is it necessary to typedef the structure on top of declaring it? In C, this:

struct foo { ... };
... sizeof(struct foo); ...

is legal, no need to typedef it.

Apart from that, I think the helper structure should be scoped in the ff_
namespace: ff_pad_structure_helper_##name (but in that case, it may not need
to be that long; not a problem though).

> +typedef struct name { \
>      __VA_ARGS__ \
> -    char reserved_padding[size - sizeof(struct { __VA_ARGS__ })];
> +    char reserved_padding[size - sizeof(pad_structure_helper_##name)]; \
> +} name;
>  
>  /**
>   * 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,

> +

Nit: no empty line between doxy and declaration.

And I wonder: does Doxygen understand the resulting construct? With the
current code, the fields are not shown, but at least the documentation for
the structure is produced:

http://ffmpeg.org/doxygen/trunk/bprint_8h.html

> +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,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141124/93fc10f7/attachment.asc>