[FFmpeg-devel] [PATCH] libavcodec/wmavoice.c: local variablepowmight mask pow function

[Axel Holzinger]

On Mon, Jul 26, 2010 at 11:18 AM, Eli Friedman wrote: 
> On Mon, Jul 26, 2010 at 1:27 AM, Axel Holzinger 
> <aholzinger at gmx.de> wrote:
> > Hi Eli et al,
> >
> > On Sun, Jul 25, 2010 at 9:41 PM, Eli Friedman wrote:
> >> On Sun, Jul 25, 2010 at 12:18 PM, Axel Holzinger 
> >> <aholzinger at gmx.de> wrote:
> >> > The issue is that a local variable pow masks a function pow and

> >> > that hurts if powf - which is used in the scope of local 
> >> > variable pow - is not a function but a macro using double
> >> > precision pow function.
> >>
> >> And I'm pointing out that such an implementation is not 
> >> correct per C99.
> >
> > I read the standard and under 7.1.4 (1) it's stated that standard 
> > headers may export function like macros. I don't see how 
> > this couldn't be correct C99. Could you please provide a reference
> > where it's stated that a function like macro is forbidden in a
> > standard header (at least for powf)?
> 
> It's okay for powf to be a macro.
> 
> > Also Unix/Posix allows powf to be implemented as a macro:
> > http://www.opengroup.org/onlinepubs/009695399/basedefs/math.h.html
> >
> >> > Besides taking the effort to apply and document a patch, what
do 
> >> > you think stands against the patch?
> >>
> >> Simply that you haven't actually pointed to why this is necessary

> >> other than the abstract possibility of a non-conforming 
> >> implementation of math.h.
> >
> > As mentioned above, I can't see why such an implementation would
be 
> > non-conforming. Can you elaborate.
> 
> Per C99, 7.1.3p1 and p2, "pow" is only reserved for use as a 
> macro name or a file-scope identifier.  Therefore, it's legal 
> to use it as the name of a local variable in any context.  A 
> specific case of this is that the construct in wmavoice.c is 
> legal C99 code, and is required to compile on any conforming 
> C99 implementation.

Perhaps you don't yet understand the problem. Yes, it is legal to use
pow as a local variable name. Yes, it is also legal that powf is
defined in <math.h> as a macro. That does not mean that the code in
wmavoice.c is required to compile. This is one of the more obscure and
ugly consequences of macros.

Specifically, if powf() is defined as a function-like macro in
<math.h>, its definition would most likely use the pow() function. You
can expect something like the following:

#define powf(a,b) ((float)pow((double)(a),(double)(b)))

It is entirely legal C99 to have this macro definition in <math.h>,
and the consequence in wmavoice.c is that the local variable pow
prevents the function pow() from being visible to the powf() macro,
leading to a compilation error.

In general, if you use a function-like macro, you have to take care
not to redefine any of the identifiers that it uses in its definition
(unless you know exactly what you're doing). As the standard does not
specify what those identifiers are, you are essentially stuffed. In
practice, you can apply common sense and avoid using those identifiers
which are most likely to create trouble.

In the light of this, pow was a bad choice for a local variable name
when using powf() right afterwards. The patch corrects that.

Regards
Axel