[FFmpeg-devel] [PATCH]Avoid an assertion failure in ff_init_vlc_sparse

Nicolas George nicolas.george at normalesup.org
Sat Jul 13 12:22:24 CEST 2013


Le quintidi 25 messidor, an CCXXI, Carl Eugen Hoyos a écrit :
> Do you prefer keeping the assert over forwarding 
> the oom error?

I do not know the code well enough to be sure this is indeed possible, but I
see the assert seems to be triggered by the ENOMEM condition introduced in
aa74810f. ENOMEM is not an internal bug but an external error condition, and
therefore can not be checked by an assert.

Therefore, the assert is now wrong, and more precisely it is a remnant of
the sloppy error checking of the original code that was partially fixed in
aa74810f. It only needs to be explained in the commit message.

(As a side note, I believe we should agree on a standard format for commit
messages and try to stick to it. Most other projects that use Git use the
"context: short impersonal sentence" format, including commits merged from
libav and a lot of commits from this side. In this particular case, that
would become something like "lavc/bitstream: replace assert with error
checks\n\nFix possible assert failures in case of OOM introduced by
aa74810f.".)

As Michael pointed out, another way of avoiding the assert failure would be
to avoid the dynamic allocation: if a reasonable upper bound can be found
for nb_codes, then it can be replaced by a local array. But I do not know
that this is actually possible, nor that it is the only code path that can
lead to an error.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130713/77c0f668/attachment.asc>


More information about the ffmpeg-devel mailing list