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.

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

> 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).

> 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 do not consider
myself such, so I put asserts, a lot of them. And I leave them after the
debug phase, and they very rarely get triggered, but that is just boasting.

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

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

> Again, lading your library code with abort() is wrong.

You can copy-paste as much as you like, only arguments will convince.

> 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.

I was talking about asserts in general, and they usually serve to check the
internal consistency of your own code. Something like that:

    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.

And you LEAVE IT THERE, because if there is a corner case that you forgot to
test and it fails, that means you probably wrote garbage beyond the end of
the buffer, and so you want your application to CRASH RIGHT NOW, not allow
remote code execution.

Regards,

-- 
  Nicolas George

Attachment: signature.asc
Description: Digital signature

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

Reply via email to