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@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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
