Skip to content

Commit

Permalink
Fixed memory leak in console variable management.
Browse files Browse the repository at this point in the history 
If _DEBUG traverse all nodes in the cvar PathDirectory when clearing
and validate algorithm logic.
  • Loading branch information
@danij-deng
danij-deng committed Aug 14, 2011
1 parent e54f6cb commit 300d93f
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 28 deletions.
77 changes: 58 additions & 19 deletions doomsday/engine/portable/src/con_data.c
View file
Expand Up @@ -118,12 +118,13 @@ static int clearVariable(struct pathdirectory_node_s* node, void* paramaters)
cvar_t* var = PathDirectoryNode_DetachUserData(node);
if(NULL != var)
{
assert(PCF_NO_LEAF == PathDirectoryNode_Type(node));
if((var->flags & CVF_CAN_FREE) && var->type == CVT_CHARPTR)
{
char** ptr = (char**) var->ptr;
// \note Multiple vars could be using the same pointer (so ensure
// that we attempt to free only once).
PathDirectory_Iterate2(cvarDirectory, PCF_NO_BRANCH, NULL, -1, markStringVariableFreed, ptr);
PathDirectory_Iterate2(cvarDirectory, PCF_NO_BRANCH, NULL, PATHDIRECTORY_PATHHASH_SIZE, markStringVariableFreed, ptr);
free(*ptr);
*ptr = emptyString;
}
Expand All @@ -134,8 +135,14 @@ static int clearVariable(struct pathdirectory_node_s* node, void* paramaters)

static void clearVariables(void)
{
PathDirectory_Iterate(cvarDirectory, PCF_NO_BRANCH, NULL, -1, clearVariable);
PathDirectory_Destruct(cvarDirectory); cvarDirectory = NULL;
/// If _DEBUG we'll traverse all nodes and verify our clear logic.
#if _DEBUG
int flags = 0;
#else
int flags = PCF_NO_BRANCH;
#endif
PathDirectory_Iterate(cvarDirectory, flags, NULL, PATHDIRECTORY_PATHHASH_SIZE, clearVariable);
PathDirectory_Destruct(cvarDirectory), cvarDirectory = NULL;
cvarCount = 0;
}

Expand Down Expand Up @@ -273,14 +280,32 @@ static boolean removeFromKnownWords(knownwordtype_t type, void* data)
return false;
}

typedef struct {
uint count;
cvartype_t type;
boolean hidden;
boolean ignoreHidden;
} countvariableparams_t;

static int countVariable(const struct pathdirectory_node_s* node, void* paramaters)
{
assert(NULL != node && NULL != paramaters);
{
countvariableparams_t* p = (countvariableparams_t*) paramaters;
cvar_t* var = PathDirectoryNode_UserData(node);
uint* count = (uint*) paramaters;
if(NULL != var && !(var->flags & CVF_HIDE))
++(*count);
if(!(p->ignoreHidden && (var->flags & CVF_HIDE)))
{
if(!VALID_CVARTYPE(p->type) && !p->hidden)
{
if(!p->ignoreHidden || !(var->flags & CVF_HIDE))
++(p->count);
}
else if((p->hidden && (var->flags & CVF_HIDE)) ||
(VALID_CVARTYPE(p->type) && p->type == CVar_Type(var)))
{
++(p->count);
}
}
return 0; // Continue iteration.
}
}
Expand All @@ -307,15 +332,19 @@ static int addVariableToKnownWords(const struct pathdirectory_node_s* node, void
*/
static void updateKnownWords(void)
{
uint c, knownCVars, knownGames;
countvariableparams_t countCVarParams;
uint c, knownGames;
size_t len;

if(!knownWordsNeedUpdate)
return;

// Count the number of visible console variables.
knownCVars = 0;
PathDirectory_Iterate2_Const(cvarDirectory, PCF_NO_BRANCH, NULL, -1, countVariable, &knownCVars);
countCVarParams.count = 0;
countCVarParams.type = -1;
countCVarParams.hidden = false;
countCVarParams.ignoreHidden = true;
PathDirectory_Iterate2_Const(cvarDirectory, PCF_NO_BRANCH, NULL, PATHDIRECTORY_PATHHASH_SIZE, countVariable, &countCVarParams);

knownGames = 0;
{ int i, gameInfoCount = DD_GameInfoCount();
Expand All @@ -327,7 +356,7 @@ static void updateKnownWords(void)
}}

// Build the known words table.
numKnownWords = numUniqueNamedCCmds + knownCVars + numCAliases + knownGames;
numKnownWords = numUniqueNamedCCmds + countCVarParams.count + numCAliases + knownGames;
len = sizeof(knownword_t) * numKnownWords;
knownWords = realloc(knownWords, len);
memset(knownWords, 0, len);
Expand All @@ -346,10 +375,10 @@ static void updateKnownWords(void)
}}

// Add variables?
if(0 != knownCVars)
if(0 != countCVarParams.count)
{
/// \note cvars are NOT sorted.
PathDirectory_Iterate2_Const(cvarDirectory, PCF_NO_BRANCH, NULL, -1, addVariableToKnownWords, &c);
PathDirectory_Iterate2_Const(cvarDirectory, PCF_NO_BRANCH, NULL, PATHDIRECTORY_PATHHASH_SIZE, addVariableToKnownWords, &c);
}

// Add aliases?
Expand Down Expand Up @@ -1374,9 +1403,11 @@ D_CMD(HelpWhat)
static int printKnownWordWorker(const knownword_t* word, void* paramaters)
{
assert(word);
{
uint* numPrinted = (void*)paramaters;
switch(word->type)
{
case WT_CCMD: {
case WT_CCMD: {
ccmd_t* ccmd = (ccmd_t*) word->data;
char* str;

Expand All @@ -1389,7 +1420,7 @@ static int printKnownWordWorker(const knownword_t* word, void* paramaters)
Con_FPrintf(CBLF_LIGHT|CBLF_YELLOW, " %s\n", ccmd->name);
break;
}
case WT_CVAR: {
case WT_CVAR: {
cvar_t* cvar = (cvar_t*) word->data;

if(cvar->flags & CVF_HIDE)
Expand All @@ -1398,18 +1429,20 @@ static int printKnownWordWorker(const knownword_t* word, void* paramaters)
Con_PrintCVar(cvar, " ");
break;
}
case WT_CALIAS: {
case WT_CALIAS: {
calias_t* cal = (calias_t*) word->data;
Con_FPrintf(CBLF_LIGHT|CBLF_YELLOW, " %s == %s\n", cal->name, cal->command);
break;
}
case WT_GAMEINFO: {
case WT_GAMEINFO: {
gameinfo_t* info = (gameinfo_t*) word->data;
Con_FPrintf(CBLF_LIGHT|CBLF_BLUE, " %s\n", Str_Text(GameInfo_IdentityKey(info)));
break;
}
}
if(numPrinted) ++(*numPrinted);
return 0; // Continue iteration.
}
}

/// \note Part of the Doomsday public API.
Expand Down Expand Up @@ -1488,21 +1521,27 @@ char* Con_GetString(const char* name)

D_CMD(ListCmds)
{
uint numPrinted = 0;
Con_Printf("Console commands:\n");
Con_IterateKnownWords(argc > 1? argv[1] : 0, WT_CCMD, printKnownWordWorker, 0);
Con_IterateKnownWords(argc > 1? argv[1] : 0, WT_CCMD, printKnownWordWorker, &numPrinted);
Con_Printf("Found %u console commands.\n", numPrinted);
return true;
}

D_CMD(ListVars)
{
uint numPrinted = 0;
Con_Printf("Console variables:\n");
Con_IterateKnownWords(argc > 1? argv[1] : 0, WT_CVAR, printKnownWordWorker, 0);
Con_IterateKnownWords(argc > 1? argv[1] : 0, WT_CVAR, printKnownWordWorker, &numPrinted);
Con_Printf("Found %u console variables.\n", numPrinted);
return true;
}

D_CMD(ListAliases)
{
uint numPrinted = 0;
Con_Printf("Aliases:\n");
Con_IterateKnownWords(argc > 1? argv[1] : 0, WT_CALIAS, printKnownWordWorker, 0);
Con_IterateKnownWords(argc > 1? argv[1] : 0, WT_CALIAS, printKnownWordWorker, &numPrinted);
Con_Printf("Found %u aliases.\n", numPrinted);
return true;
}
9 changes: 4 additions & 5 deletions doomsday/engine/portable/src/con_main.c
View file
Expand Up @@ -405,14 +405,13 @@ void Con_Shutdown(void)
{
Con_Message("Shuting down the console...\n");

Con_DestroyBuffer(histBuf); // The console history buffer.
Con_DestroyBuffer(oldCmds); // The old commands buffer.

Con_ClearExecBuffer();
Con_ShutdownDatabases();
if(prbuff)
M_Free(prbuff); // Free the print buffer.

Con_ClearExecBuffer();
Con_ShutdownDatabases();
Con_DestroyBuffer(histBuf); // The console history buffer.
Con_DestroyBuffer(oldCmds); // The old commands buffer.
}

boolean Con_IsActive(void)
Expand Down
4 changes: 2 additions & 2 deletions doomsday/engine/portable/src/filedirectory.c
View file
Expand Up @@ -123,7 +123,7 @@ static filedirectory_t* addPaths(filedirectory_t* fd, const ddstring_t* const* p
{
if(PT_BRANCH == PathDirectoryNode_Type(node))
{
PathDirectory_Iterate2_Const(fd->_pathDirectory, PCF_MATCH_PARENT, node, -1, callback, paramaters);
PathDirectory_Iterate2_Const(fd->_pathDirectory, PCF_MATCH_PARENT, node, PATHDIRECTORY_PATHHASH_SIZE, callback, paramaters);
}
else
{
Expand Down Expand Up @@ -274,7 +274,7 @@ static void clearNodeInfo(filedirectory_t* fd)
{
assert(NULL != fd);
if(NULL == fd->_pathDirectory) return;
PathDirectory_Iterate(fd->_pathDirectory, 0, NULL, -1, freeNodeInfo);
PathDirectory_Iterate(fd->_pathDirectory, 0, NULL, PATHDIRECTORY_PATHHASH_SIZE, freeNodeInfo);
}

void FileDirectory_Destruct(filedirectory_t* fd)
Expand Down
11 changes: 9 additions & 2 deletions doomsday/engine/portable/src/pathdirectory.c
View file
Expand Up @@ -159,6 +159,13 @@ static void clearPathHash(pathdirectory_pathhash_t* ph)
while(NULL != (*ph)[i])
{
pathdirectory_node_t* next = (*ph)[i]->next;
#if _DEBUG
if(NULL != PathDirectoryNode_UserData((*ph)[i]))
{
Con_Error("PathDirectory::clearPathHash: Node %p has non-NULL user data.", (*ph)[i]);
exit(1); // Unreachable.
}
#endif
PathDirectoryNode_Destruct((*ph)[i]);
(*ph)[i] = next;
}
Expand Down Expand Up @@ -373,7 +380,7 @@ static int iteratePaths(pathdirectory_t* pd, int flags, pathdirectory_node_t* pa
{
pathdirectory_node_t* node;
ushort i;
if(parent == NULL && hash >= 0 && hash < PATHDIRECTORY_PATHHASH_SIZE)
if(hash < PATHDIRECTORY_PATHHASH_SIZE)
{
for(node = (*pd->_pathHash)[hash]; NULL != node; node = node->next)
{
Expand Down Expand Up @@ -417,7 +424,7 @@ static int iteratePaths_const(const pathdirectory_t* pd, int flags, const pathdi
{
pathdirectory_node_t* node;
ushort i;
if(parent == NULL && hash >= 0 && hash < PATHDIRECTORY_PATHHASH_SIZE)
if(hash < PATHDIRECTORY_PATHHASH_SIZE)
{
for(node = (*pd->_pathHash)[hash]; NULL != node; node = node->next)
{
Expand Down

0 comments on commit 300d93f

Please sign in to comment.