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

[Axel Holzinger]

On Mon, Jul 26, 2010 at 12:56 AM, M?ns Rullg?rd wrote:
> "Axel Holzinger" <aholzinger at gmx.de> writes:
> 
> > 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
> >> >> > 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>,
> 
> No, it is not.  Since 'pow' is not reserved for identifers 
> with local scope, the header may not use it in any way which 
> might be incompatible with such use of the identifier in 
> application code.  If math.h were to implement powf() as a 
> macro, it would have to define
> __pow() as an alias for pow() and use this in the macro definition.
> Identifiers starting with __ (double underscore) are reserved 
> for any use, so this would not cause problems for any 
> conforming application code.

Fair enough, could you please point me to clause in the standard
that documents this?

> > 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.
> 
> Did this actually happen to you, or are you simply trolling?

As you may have already have noticed from other threads, I'm currently
trying to build FFmpeg on MinGW32 with icl (Intel compiler for
windows)
as compiler. As Intel doesn't bring a complete library, but instead
fall back on some of the MS headers, math.h of MS is used.

In the x64 part powf is a function and in the x86 part powf is a macro
using the double precision version pow.

Now, if you are right that would be illegal for a standard lib header
(not only for C99, but also for C89, which MS claims to be compatible
with). Still they do - and belieive me I know your reluctance
regarding
MS - the patch doesn't add any code cluttering or #ifdef orgy, but
solves
this issue and potentially others.

> > 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.
> 
> The standard _does_ reserve the __* namespace for 
> implementation use, so those are the names such macros must 
> use.  It is perfectly safe and predictable.

I just looked at MinGW's math.h:
#define isfinite(x) ((fpclassify(x) & FP_NAN) == 0)

This would then also be illegal, because the usage of fpclassify would
mask a local variable fpclassify.

On my Kubuntu 9.04 in math.h I find

#  define isgreater(x, y) \
  (__extension__
   ({ __typeof__(x) __x = (x); __typeof(y) __y = (y);
      !isunordered (__x, __y) && __x > __y; }))

This masks a local variable isunordered.

Regards
Axel