On Sun, Aug 23, 2015 at 12:04 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > On Fri, Aug 21, 2015 at 6:21 AM, Carl Eugen Hoyos <ceho...@ag.or.at> wrote: >> Hi! >> >> Attached patch fixes a memleak for me, reproducible with: >> $ valgrind --leak-check=full ./ffmpeg_g -i >> fate-suite/lena.pnm -reset_timestamps 1 -f segment out%1d.avi > > It fixes a memleak for me, but note that valgrind output is still not clean: > 3 errors from 2 contexts becomes > 2 errors from 1 context.
I now understand valgrind a bit better, so the other leak is not part of this discussion. Nevertheless, it should be investigated. > > Are you sure nothing else needs to be freed? > I am unfortunately not familiar with this code at all, > so I don't know what needs to be done. I probed the code a little more, To me, there is another questionable line in 350, where a memcpy is done. I have no idea why when a linked list is used a memcpy needs to be done for a new entry. It is inefficient (extra alloc and free), and is not needed: just use the already allocated object's pointer. Can you clarify this? Patch itself looks ok, but I would hold off until the above can be explained and changed if needed. However, if it needs changing, that should belong to a separate patch. > >> >> Please review, Carl Eugen >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
