[FFmpeg-devel] [PATCH] do not #include assert.h directly

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Tue, May 13, 2008 at 11:49:18PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Tue, May 13, 2008 at 07:47:56PM +0100, Mans Rullgard wrote:
>> >> libavutil/internal.h includes assert.h, defining NDEBUG if DEBUG is
>> >> not defined.  Including assert.h again has no effect, and is possibly
>> >> confusing.
>> >
>> > IMHO internal.h should not #include assert.h
>> 
>> I figured it might be a good idea to have the same assert() behaviour
>> (enabled/disabled) everywhere.  Either way, it should be consistent.
>
> Well, the asserts fall in several different categories.
> 1. Ones in speed critical code
> 2. Ones related to security
> 3. Remainder
>
> 1. should generally be disabled unless one needs them for debuging
> 2. should generally be enabled unless one knows what he is doing
>    (i think we do prefer redundant checks over exploits ...)
> 3. should probably be enabled unless CONFIG_SMALL is defined

I can agree with that.

> Because of the possibility of asserts() falling in category 2. I do not
> feel well with throwing the cases out which unconditionally enabled
> asserts. At least not without some reviewing of all affected asserts but
> i do not volunteer for that.

The problem right now is that very few files have assert() enable, due
to internal.h disabling them before assert.h is included a second
time.

Perhaps we should define some helper macros for the different
categories of assert() usage.

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