[FFmpeg-devel] [PATCH] fix compilation (was Re: r12489 broke all builds)

[Måns Rullgård]

Reimar D?ffinger wrote:
> On Wed, Mar 19, 2008 at 08:38:43AM +0000, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> > The cleanest approuch is to never include headers from other headers.
>>
>> Wherever did you get that notion?  It is pure madness.  If you
>> disagree, come visit me at my dayjob, where some madmen operated under
>> that rule for years.  The resulting chaos is hard to imagine.
>
> I have no problem believing the chaos, but I doubt that header inclusion
> rules are the reason. At least I can't see that much of MPlayer's chaos
> would have come from _that_ rule.

I didn't even know mplayer had such a rule.  Anyhow, it certainly hasn't
helped mplayer code quality.

>> > But i suspect that i, ivan and arpi are in the minority with that view.
>>
>> I didn't know you were that many.
>
> Well, I do not like headers including everything too much either,
> because it makes it less clear which headers one is supposed to include.

Every file should include all headers it explicitly uses symbols from.
The only exception is when another header is documented to include
a required header, e.g. inttypes.h includes stdint.h.

> As an example (that probably can not happen in exactly this way, so just
> to give you an idea of what I think of) a file might include bswap.h,
> then later adds endian dependent code under #ifdef WORDS_BIGENDIAN.
> Later then someone removes the bswap.h include and suddenly
> WORDS_BIGENDIAN never gets defined since nobody included any other
> header - since it was not necessary, due to bswap.h including
> everything, stdint.h, common.h, config.h,...

By the above rule, any file that uses WORDS_BIGENDIAN should explicitly
include config.h.  It so happens, that common.h is (or should be)
documented to include config.h (when building FFmpeg), and avcodec.h
can be relied on to include common.h.  However, there is no promise
that bswap.h include any additional headers.

A more realistic example is a file, let's call it mpeg12data.c, that includes,
say, rational.h, and everything is fine.  One fine day, someone changes
rational.h to require stuff from common.h, without adding the #include
line.  Suddenly, mpeg12data.c fails to compile because it doesn't include
common.h, and has no reason to do so, not directly using any of its symbols.

It is not reasonable that backwards compatible changes to a header file
should require updating every source file that uses.  Just think for a
moment about public API headers.

-- 
M?ns Rullg?rd
mans at mansr.com