[FFmpeg-devel] [PATCH] avcodec/cbs: Avoid leaving the ... out in calls to variadic macros

[Andreas Rheinhardt]

Mark Thompson:
> On 23/03/2020 00:52, Andreas Rheinhardt wrote:
>> According to C99, there has to be at least one argument for every ...
>> in a variadic function-like macro. In practice most (all?) compilers also
>> allow to leave it completely out, but it is nevertheless required: In a
>> variadic macro "there shall be more arguments in the invocation than there
>> are parameters in the macro definition (excluding the ...)." (C99,
>> 6.10.3.4).
>>
>> CBS (not the framework itself, but the macros used in the
>> cbs_*_syntax_template.c files) relies on the compiler allowing to leave
>> a variadic macro argument out. This leads to warnings when compiling in
>> -pedantic mode, e.g. "warning: must specify at least one argument for
>> '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]"
>> from Clang.
>>
>> Most of these warnings can be easily avoided: The syntax_templates
>> mostly contain helper macros that expand to more complex variadic macros
>> and these helper macros often omit an argument for the .... Modifying
>> them to always expand to complex macros with an empty argument for the
>> ... at the end fixes most of these warnings: The number of warnings went
>> down from 400 to 0 for cbs_av1, from 1114 to 32 for cbs_h2645, from 38 to
>> 0 for cbs_jpeg, from 166 to 0 for cbs_mpeg2 and from 110 to 8 for cbs_vp9.
>>
>> These eight remaining warnings for cbs_vp9 have been fixed by switching
>> to another macro in cbs_vp9_syntax_template: The fixed values for the
>> sync bytes as well as the trailing bits for byte-alignment are now read
>> via the fixed() macro (this also adds a check to ensure that trailing
>> bits are indeed zero as they have to be).
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>> There are two ways to fix the remaining 32 warnings from cbs_h2645:
>>
>> Simply add ", " to all macro calls that make use of the complex macros;
>> this has the drawback of adding uglyness to cbs_h26x_syntax_template.c.
>>
>> Or add new macros for these macro calls: The places that produce
>> warnings use the complex macros directly, because they use names
>> different from the default names that the helper macros use, but they do
>> not use subscripts and therefore leave the variadic argument (designed
>> for subscripts) out. I would have implemented the second solution if it
>> were not for the problem of the naming of the new macros.
>>
>> (There is of course also the possibility not to care about the remaining
>> ones.)
>>
>>  libavcodec/cbs_av1.c                 | 16 ++++++++--------
>>  libavcodec/cbs_h2645.c               | 14 +++++++-------
>>  libavcodec/cbs_jpeg.c                |  2 +-
>>  libavcodec/cbs_mpeg2.c               |  6 +++---
>>  libavcodec/cbs_vp9.c                 | 13 ++++++-------
>>  libavcodec/cbs_vp9_syntax_template.c | 21 ++++-----------------
>>  6 files changed, 29 insertions(+), 43 deletions(-)
> 
> Looks fine to me, keeping the ugliness in the macros rather than the templates is good.
> 
> Is there any compiler which actually fails here, or is the only case which finds it the warning in clang pedantic mode?
> 
Both Clang as well as GCC warn in pedantic mode for this (although GCC's
warning is not called "gnu-zero-variadic-macro-arguments"; it simply
says "warning: ISO C99 requires at least one argument for the "..." in a
variadic macro").

Any suggestions for macro names for the remaining stuff (stuff that uses
an unusual name, but no subscripts (like last_payload_type_byte and
last_payload_size_byte))? If not, I'll apply.

I know of no compiler that refuses to compile because of this.

- Andreas