[MPlayer-cvslog] r27288 - trunk/configure

[Uoti Urpala]

On Tue, 2008-07-15 at 21:20 +0200, Reimar Döffinger wrote:
> > "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)?
> 
> Well, ao_alsa needs the alsa header to work with sys/time.h.

Yes, but by that same logic we could add all kinds of random headers
that might be included in the same file(s) "because it needs to work
with them". That they conflict in one already broken case is IMO not
enough justification to add the unrelated header to the check; and any
headers added to configure checks for such reasons should be clearly
commented.

> Though I do not see why it is needed at all right now, none of the stuff from time.h
> is used and current alsa headers include time.h. Does someone have some
> really old alsa versions around to test if it is ever needed?

What functions legitimately need <sys/time.h> in MPlayer in general?
gettimeofday() and legacy includes for select() (instead of select.h)
are the only ones I can think of, anything else?

> > 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.
> 
> Well, obviously we have different ideas of what configure is supposed to
> do. I think it should check which features actually _work_ and enable
> those and not just enable those that somehow if you are not too unlucky
> could work.

I'm not sure why you thought the role of configure would be significant
for this particular case, but anyway I think configure should not try to
do any in-depth checks to silently work around existing but somehow
broken features. In case it tests for a common problem that should be
explicitly printed instead of just saying "no" for the feature.

In this particular case that however seems irrelevant. What would you
test anyway? You said you'd add the alloca.h header to the check. What
would be the point? It'd make no difference since there is no reference
to alloca() unless the compiled file uses the macros, and the configure
test doesn't. And actually testing the macros in configure doesn't seem
any more useful than testing any other feature of the headers.

> Of course there are corner cases that are too unlikely and too much
> effort to test and/or maintain to be worth testing, but that is a
> different point.
> That said, does someone remember why we are using that alloca mess
> instead of the proper malloc variants? Weren't they available in earlier
> versions or something?

Using malloc with explicit free() would at least make it more
complicated and error-prone.