[vlc-devel] [PATCH 3/3] subtitle-demux: fix memory leaks (fixes #11908)

Tristan Matthews le.businessman at gmail.com
Wed Sep 10 21:31:22 CEST 2014


On Wed, Sep 10, 2014 at 2:52 PM, Hannes Domani <ssbssa at yahoo.de> wrote:
> Tristan Matthews <le.businessman at gmail.com> schrieb am 20:08 Mittwoch, 10.September 2014:
>> On Wed, Sep 10, 2014 at 12:25 PM, Hannes Domani <ssbssa at yahoo.de> wrote:
>> > Tristan Matthews <le.businessman at gmail.com> schrieb am 18:20 Mittwoch, 10.September 2014:
>> >> On Wed, Sep 10, 2014 at 12:17 PM, Hannes Domani <ssbssa at yahoo.de> wrote:
>> >> > Tristan Matthews <le.businessman at gmail.com> schrieb am 17:43 Mittwoch, 10.September 2014:
>> >> >> >
>> >> >> > But I still think asprintf() is a bad choice here.
>> >> >>
>> >> >> Have you profiled to see what kind of impact it has?
>> >> >
>> >> > On Windows, with the example of #11908 it originally
>> >> > took more than 11 minutes to start playing.
>> >> > With my changes it takes 3 seconds to start playing.
>> >>
>> >> Do you mean without asprintf or without the memory leak? I was asking
>> >> about without asprintf.
>> >
>> > Yes, with asprintf -> 11 minutes.
>> > Without asprintf -> 3 seconds.
>>
>> OK, would it be possible to refactor the allocation(s) out of the loop
>> and only resize if needed?
>
> Not sure how you mean that.
> We don't know before how big the buffer needs to be.

Sorry, your initial patch was already doing what I meant by using
realloc_or_free instead of asprintf.
My only other issue with it was that it would be nicer to use an
snprintf followed by header_len += s_len + 1 instead of the two
memcpys and header_len additions....unless you're finding that this is
slower.

Best,
Tristan



More information about the vlc-devel mailing list