[FFmpeg-cvslog] r16746 - trunk/libavcodec/rectangle.h

[Michael Niedermayer]

On Sun, Jan 25, 2009 at 04:42:14AM +0000, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > On Sat, Jan 24, 2009 at 03:30:36PM +0000, M?ns Rullg?rd wrote:
> >> Diego Biurrun <diego at biurrun.de> writes:
> >> 
> >> > On Sat, Jan 24, 2009 at 02:51:22PM +0000, M?ns Rullg?rd wrote:
> >> >> diego <subversion at mplayerhq.hu> writes:
> >> >> 
> >> >> > Log:
> >> >> > Add required headers to fix 'make checkheaders'.
> >> >> >
> >> >> > --- trunk/libavcodec/rectangle.h	Sat Jan 24 15:32:22 2009	(r16745)
> >> >> > +++ trunk/libavcodec/rectangle.h	Sat Jan 24 15:46:00 2009	(r16746)
> >> >> > @@ -28,7 +28,10 @@
> >> >> >  #ifndef AVCODEC_RECTANGLE_H
> >> >> >  #define AVCODEC_RECTANGLE_H
> >> >> >
> >> >> > +#include <assert.h>
> >> >> 
> >> >> Be careful with adding assert.h.  You just enabled assert() checking
> >> >> unconditionally in that file and anything that includes it.  Before
> >> >> this commit, assert.h was included through internal.h, which sets
> >> >> NDEBUG before including it.
> >> >
> >> > What is the proper solution then?  Adding #define NDEBUG or #including
> >> > internal.h instead?
> >> 
> >> I don't know.  Michael refused to talk about it last time I brought it
> >> up.
> >
> > huh?
> > i suggested a solution and was entirely ignored by everyone
> >
> > The problem being that there are different types of asserts, some
> > * being in speed critical code, these we dont want enabled unless realy needed
> > * being about security related checks like (we know it fits in the buffer but
> >   lets put an assert there anyway ...)
> > * being neither of above
> >
> > if these 3 are split, we can handle things globally instead of per file.
> > But currently
> > Enabling everything globally means a huge speedloss and is unacceptable
> > Disabling everything globally means debuging will be harder as users have
> > them disabled and it might lead to secholes as well as missing bugs
> > because the asserts that would give hints where disabled.
> 
> That means 3 different macros wrapping assert().
> 
> Examining all existing assert() calls (638 at the moment) and picking
> the right version is a lot of work.  How about we add the required
> macros now, and require new code to use the appropriate one.  That
> will at least make sure the problem doesn't grow.
> 
> So what do we call the wrappers?  I suggest something like this:
> 
> - ff_assert_fast()     for speed-critical code
> - ff_assert_critical() for security related checks
> - ff_assert()          for everything else
> 
> For the non-critical ones, we need a controlling configure setting.

no objections

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090125/d9cae8e6/attachment.pgp>