[Ffmpeg-cvslog] r5740 - in trunk/libavutil: Makefile lls.c lls.h

[Rich Felker]

On Fri, Jul 14, 2006 at 07:00:11PM +0200, Michael Niedermayer wrote:
> > Yet there are no asserts in this file.
> 
> yes, ... ive already removed these 2 lines ...

Yes I noticed after sending the message.

> > Also I think this is inappropriate practice. There may be a reason

I should clarify I think. I didn't mean that using assert is
inappropriate in itself, but rather that overriding the build
process's definition of NDEBUG is inappropriate since NDEBUG signals
that the person building does not want asserts.

> i think i diagree here, if a assert() fails then the following behavior
> is random, after all the developer wrote that part of the code with the
> strong belive his assert() would be true, so how can a failing assert()
> be worse then completely undefined behavior (segfault, exploit, data loss,
> ...)

agree. i was more thinking about linking considerations. maybe no one
cares because ffmpeg is already so big, but if nothing else, each
assert uses __func__ which causes an extra string (the function name)
to be included in the object file, as well as the code to call assert.

> > Of course whenever you use assert it should be provable that the
> > assert is never failable anyway, 
> 
> well, if i take that litterally then asserts where completely useless
> because they could never fail ...

well there are 3 cases:

1. lazy developer puts assert for something that should not happen but
   clearly can happen with corrupt input files. this is obviously very
   bad for a library to do.

2. developer puts assert for something that should be impossible, but
   for which he/she has not sufficiently verified the correctness of
   the code or debugged, probably hoping that an assertion failure
   will happen and signal the existence of a bug if the code actually
   is wrong. probably not a bad idea, but indicates that the code
   needs detailed auditing at some point.

3. assert is placed exclusively as a comment that the condition must
   be true, and it's so obvious that even the compiler can catch it
   and optimize out the assert. i don't have much of an opinion on
   whether this is good or not but i think i'd prefer just a comment.

> i would rather say that assert(x) is a statement from the developer that
> he is certain that x is true, but he is just a human and might be wrong
> either due to bugs, later changes or simple oversight of some corner case

yes, guarding against later changes is another very good use. i just
tend to think it should still respect NDEBUG.

rich