From af403073f055323ce72efcce4e990c0c52743c92 Mon Sep 17 00:00:00 2001 From: Christophe CURIS Date: Wed, 8 May 2013 15:44:12 +0200 Subject: [PATCH] Fixed memory leak due to non-freed property list structure The history is actually loaded from a file into a property list that is then converted to an array. The intermediate property list was not freed, which led to memory leak. It looks like it was a tentative of optimisation to avoid duplicating an already allocated string and re-use the pointer instead, but this means it is not possible to free the original container as it would free the string too. There is a better way to do this, but it requires an API change on the WUtil library so it is left for a future improvment. --- src/dialog.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/dialog.c b/src/dialog.c index 05087474..7b0b4f28 100644 --- a/src/dialog.c +++ b/src/dialog.c @@ -191,24 +191,38 @@ static WMArray *LoadHistory(const char *filename, int max) WMPropList *plitem; WMArray *history; int i, num; + char *str; history = WMCreateArrayWithDestructor(1, wfree); WMAddToArray(history, wstrdup("")); plhistory = WMReadPropListFromFile(filename); - if (plhistory && WMIsPLArray(plhistory)) { - num = WMGetPropListItemCount(plhistory); + if (plhistory) { + if (WMIsPLArray(plhistory)) { + num = WMGetPropListItemCount(plhistory); - for (i = 0; i < num; ++i) { - plitem = WMGetFromPLArray(plhistory, i); - if (WMIsPLString(plitem) && WMFindInArray(history, strmatch, - WMGetFromPLString(plitem)) == WANotFound) { - WMAddToArray(history, WMGetFromPLString(plitem)); - if (--max <= 0) - break; + for (i = 0; i < num; ++i) { + plitem = WMGetFromPLArray(plhistory, i); + if (WMIsPLString(plitem)) { + str = WMGetFromPLString(plitem); + if (WMFindInArray(history, strmatch, str) == WANotFound) { + /* + * The string here is duplicated because it will be freed + * automatically when the array is deleted. This is not really + * great because it is already an allocated string, + * unfortunately we cannot re-use it because it will be freed + * when we discard the PL (and we don't want to waste the PL's + * memory either) + */ + WMAddToArray(history, wstrdup(str)); + if (--max <= 0) + break; + } + } } } + WMReleasePropList(plhistory); } return history;