Re: [dev] Re: [dmenu] [patch] add xft and fix possible memory leak in version 4.2.1

On 21 November 2010 14:02, Connor Lane Smith <cls_AT_lubutu.com> wrote:
> On 21 November 2010 03:56, Dan Brown <danbrown_AT_gmail.com> wrote:
>> 2) dmenu v4.2.1 appears to be leaking memory. It is missing the
>> routines to teardown/cleanup memory structures present in previous
>> versions. This patch also adds them.
>
> As Wolf says, there is no memory leak: there are only mallocs right at
> the start, and no opportunities to free until the moment before dmenu
> exits (which tears down the entire process space anyway).
>
> On 21 November 2010 08:11, Anselm R Garbe <garbeam_AT_gmail.com> wrote:
>> Well for clarity and consistency reasons I prefer if dmenu cleans up
>> all resources acquired at the end. So that cleanup() was removes looks
>> more like a bug to me and thanks to Dan for pointing this out.
>
> It was intentional. It is not even possible for there to be a memory
> leak, so all we get is a slower dmenu with more complex code, with
> absolutely no benefit. The more items there are the slower it gets,
> and most of the heap doesn't get freed until exit anyway. cleanup() is
> like sweeping the floor for the apocalypse.

I prefer to keep cleanup() even if it slows down the performance
(which I doubt will be noticeable) just for the sake of keeping the
symmetry that the code that allocates resources also deallocates them.
This might sound a bit pedantic and useless in case of dmenu, but I
prefer to be consistent here.

Cheers,
Anselm
Received on Mon Nov 22 2010 - 13:01:14 CET