[MPlayer-cvslog] r27288 - trunk/configure

[Uoti Urpala]

On Tue, 2008-07-15 at 20:07 +0200, Reimar Döffinger wrote:
> On Tue, Jul 15, 2008 at 08:00:02PM +0300, Uoti Urpala wrote:
> > While this commit probably does little practical harm I still dislike
> > adding unrelated includes like <sys/time.h> to configure tests.
> 
> How is this unrelated, it tests _exactly_ what ao_alsa.c needs.

"Exactly what ao_alsa.c needs"? How so? How is <sys/time.h> related at
all except that you saw a broken header conflict in one case (and a case
with broken compilation settings anyway)?

It's "closer to what the file needs" in the sense that the test code is
closer (by 1 line) to compiling the actual file, but IMO that alone is
not a good reason to do it. We could add all headers that might be
included together with the tested one or even try compiling the full
files where the header might be used, but that would obviously be silly.
That a certain system had a header conflict in a broken-anyway case is
IMO not a sufficient reason to add an otherwise unrelated header just to
see if it breaks something.

Also in a case where a conflict check was warranted there should
definitely be a comment in configure explaining why an apparently
unrelated-looking header is being included in the test.

> And actually I would prefer to also add another header it also needs but
> forgets to include: alloca.h

The timeval stuff is not about "a header it needs but forgets to
include". It does include <time.h>; the timeval definition is a (broken)
workaround for the already broken case where <time.h> doesn't define
struct timeval because of missing standard #defines.

At least the alloca uses I noticed were in macro definitions only, and
so only matter if the source file uses the snd_*_alloca() macros. Since
the configure test does not use those macros it does not need alloca.h.