[FFmpeg-devel] [PATCH] libavutil: AVEncodeInfo data structures

[Nicolas George]

James Almer (12019-08-13):
> I'm fairly sure this was discussed before, but invalid arguments
> shouldn't crash an user's application. They even have their own
> standardized errno value for this purpose.
> asserts() are to catch bugs in our code, not in theirs. Returning a NULL
> pointer is the preferred behavior.

I do not agree. Asserts are to catch all bugs that can be catched
statically. There is no sense in making some bugs harder to catch just
because they involve separate developers.

Nobody can predict whether a disk will make a I/O error, whether there
will be enough memory, etc., that kind of error MUST be handled
gracefully.

On the other hand, it is easy to make sure that a buffer given to read()
is not NULL, and therefore it is very acceptable to just crash when that
happens.

EINVAL is for cases where the acceptable value cannot be easily
predicted by the caller. For example, setting an unsupported sample rate
for the sound device: the caller could check in advance, but that would
be cumbersome.

Now, please tell me, according to you, "idx is not smaller than
nb_blocks", is it more akin to a disk I/O error, a NULL buffer or an
invalid sample rate?

Another argument:

Instead of providing utility code, we could just document the
arithmetic. In that case, the application would have code that says, in
essence: "block = info + offset + idx * block_size". No check. What
would happen if idx was too big? Not a graceful error: a crash, or
worse.

The assert mimics that, in a more convenient way since it gives the
reason and line number, and does not allow the bug to devolve into
something more serious than a crash.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190814/73f567bb/attachment.sig>