On 23/03/15 16:01, Nicolas George wrote:
Le tridi 3 germinal, an CCXXIII, Luca Barbato a écrit :
Only if you are not managing the failure properly.

You can not manage the failure properly if you can not trigger the
failure to test it.

Thus you assume that the best way to manage it is by crashing...

And managing properly a failure that can not actually happen is just
dead code making the whole useful code less readable.

You are putting a  `if (!condition) abort()`, this in itself is a crash.

So you are adding a crash that might or might not happen instead of a normal failure path that boils down usually to a

    if (!condition) return foo;

Keep in mind that asserts do get compiled out.

Sometimes. lavu has av_assert0(), which by default is enabled even
for installed release code, and av_assert1() which is disabled by
default (in FFmpeg we have also av_assert2() for code that is even
more speed-critical, and a configure option to easily select the
level).

I do care about Deny Of Service and that's why I'm slowly removing all of this.

No it is better to have your user get a message "bug catched" and
have the server or client working just fine beside not having that
audio stream.

Well, yes. In an ideal world with only perfect developers.

I like to consider actual cases like the one at hand.

No, you are just putting some easy to trigger deny of service.

A deny of service is better than an arbitrary code execution.

I hope we both want none of them and you can _AVOID_ adding a deny of service to begin with.

In this specific case

In this specific case, that entirely depends on the behaviour of the
library. I am usually not fond of asserts for values returned by a
library, because this is an external circumstance that can change,
not a bug from my code. It all depends on what the library
documentation states and how much confidence we can have it will
respect it. The best person to decide that seems to me the person
submitting the patch.

No, there is no "best person", something is either acceptable or not.

Either you trust your input or you validate it.

You do not crash willingly on it if is wrong.

buffer_size = something + something else + number * elsize + ... p =
buffer = malloc(buffer); put_something(&p); put_something_else(&p);
... assert(p - buffer == buffer_size);

You put it in your original code to be sure that your buffer_size
formula is correct and test it intensively.

This is what you do with unit tests (and cases like that are prevented completely thanks nice abstractions such as avio...).

You are in a false dichotomy, you want to have a crash of a specific kind instead of another because you know that your code is/might be wrong.

You can just not have the problem to begin with.

I do not want to have a **crash** or something arguably worse to begin with.

lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to