On Sun, 1 Jun 2014, Derek Buitenhuis wrote:
On 6/1/2014 6:19 PM, Luca Barbato wrote:
The code itself looks fine, you might factorize the deallocation with a
"goto fail;".
No.
On 6/1/2014 1:10 PM, Nidhi Makhijani wrote:
- if (!hdr)
+ if (!hdr) {
+ int j;
+ for (j=1; j < i; j++)
+ av_freep(&nut->header_len[j]);
+ av_freep(nut->time_base);
Either:
av_freep(&nut->time_base);
or
av_free(nut->time_base);
nut->stream = av_mallocz(sizeof(StreamContext) * stream_count);
+ if (!nut->stream) {
+ int j;
+ for (j=1; j<nut->header_count; j++)
+ av_freep(&nut->header_len[j]);
+ av_freep(nut->time_base);
+ return AVERROR(ENOMEM);
+ }
Ditto.
For code like this, it's imperative that the author of the patches
actually test run the error cleanup codepaths (in valgrind or something
similar). Error handling codepaths that aren't normally exercised can
easily be arbitrarily broken, which will only be found once someone runs
fuzzing the next time.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel