[FFmpeg-cvslog] r16746 - trunk/libavcodec/rectangle.h
Sun Jan 25 10:08:35 CET 2009
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.
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
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-cvslog