From a2724ace243e91f45310697eafaafe8d44245242 Mon Sep 17 00:00:00 2001 From: H.G. Muller Date: Fri, 6 May 2011 19:17:56 +0200 Subject: [PATCH] Plug memory leak, filenames relative to installDir String options are consistently freed now, before assigning a new value to them through strdup. The init code now even does use strdup when setting defaults. This is important for optionslike -lgf, which are not saved in the settings file, and thus are usually left with their default. When a dialog to set them then uses free, this causes a crash. When setting the defaults uses strdup, ParseArgs can already free them. This plugs an important memory leak, as ParseArgs in now called to parse the tourney file before every tourney game (twice!), and the lists of participants and results in it can be quite long. Macros are defined in common.h to make the code look simpler. Filenames entered through the generic popup are now interpreted relative to the WinBoard installation folder, just as the saveGameFile already was. The code handling the atter was made into a subroutine for this. --- args.h | 7 ++++--- common.h | 4 ++++ frontend.h | 2 ++ winboard/winboard.c | 2 ++ winboard/woptions.c | 43 ++++++++++++++++++++++++++----------------- winboard/wsettings.c | 5 +++-- 6 files changed, 41 insertions(+), 22 deletions(-) diff --git a/args.h b/args.h index 2471d13..c0e569b 100644 --- a/args.h +++ b/args.h @@ -792,7 +792,7 @@ ParseSettingsFile(char *name, char **addr) f = fopen(fullname, "r"); if (f != NULL) { if (addr != NULL) { - *addr = strdup(fullname); + ASSIGN(*addr, fullname); } ParseArgs(FileGet, f); fclose(f); @@ -993,7 +993,7 @@ ParseArgs(GetFunc get, void *cl) case ArgString: case ArgFilename: - *(char **) ad->argLoc = strdup(argValue); + ASSIGN(*(char **) ad->argLoc, argValue); break; case ArgSettingsFilename: @@ -1158,7 +1158,8 @@ SetDefaultsFromList() case ArgString: case ArgFilename: case ArgSettingsFilename: - *(char **) argDescriptors[i].argLoc = (char *)argDescriptors[i].defaultValue; + if((char *)argDescriptors[i].defaultValue) + *(char **) argDescriptors[i].argLoc = strdup((char *)argDescriptors[i].defaultValue); break; case ArgBoardSize: *(int *) argDescriptors[i].argLoc = (int)(intptr_t)argDescriptors[i].defaultValue; diff --git a/common.h b/common.h index 6140d5e..791e5e5 100644 --- a/common.h +++ b/common.h @@ -732,6 +732,10 @@ extern WindowPlacement wpTags; extern int chatCount; extern char chatPartner[MAX_CHAT][MSG_SIZ]; +// [HGM] generally useful macros; there are way too many memory leaks... +#define FREE(x) if(x) free(x) +#define ASSIGN(x, y) if(x) free(x); x = strdup(y) + // [HGM] for now we use the kludge to redefine all the unstructured options by their array counterpart // in due time we would have to make the actual substitutions all through the source diff --git a/frontend.h b/frontend.h index 2263fde..f498bc4 100644 --- a/frontend.h +++ b/frontend.h @@ -109,6 +109,7 @@ void EchoOn P((void)); void EchoOff P((void)); void Raw P((void)); void Colorize P((ColorClass cc, int continuation)); +char *InterpretFileName P((char *name, char *dir)); char *UserName P((void)); char *HostName P((void)); @@ -172,6 +173,7 @@ void GLT_AddToList( char *name ); Boolean GLT_GetFromList( int index, char *name ); extern char lpUserGLT[]; +extern char homeDir[]; /* these are in wgamelist.c */ void GameListPopUp P((FILE *fp, char *filename)); diff --git a/winboard/winboard.c b/winboard/winboard.c index 67e950a..0ff5e0b 100644 --- a/winboard/winboard.c +++ b/winboard/winboard.c @@ -156,6 +156,7 @@ char *programName; char *settingsFileName; Boolean saveSettingsOnExit; char installDir[MSG_SIZ]; +char homeDir[MSG_SIZ]; int errorExitStatus; BoardSize boardSize; @@ -989,6 +990,7 @@ InitInstance(HINSTANCE hInstance, int nCmdShow, LPSTR lpCmdLine) } else { GetCurrentDirectory(MSG_SIZ, installDir); } + safeStrCpy(homeDir, installDir, MSG_SIZ); gameInfo.boardWidth = gameInfo.boardHeight = 8; // [HGM] won't have open window otherwise screenWidth = screenHeight = 1000; // [HGM] placement: kludge to allow calling EnsureOnScreen from InitAppData InitAppData(lpCmdLine); /* Get run-time parameters */ diff --git a/winboard/woptions.c b/winboard/woptions.c index 801fe04..9aa820f 100644 --- a/winboard/woptions.c +++ b/winboard/woptions.c @@ -135,6 +135,28 @@ VOID SetLoadOptionEnables(HWND hDlg); VOID SetSaveOptionEnables(HWND hDlg); VOID SetTimeControlEnables(HWND hDlg); +char * +InterpretFileName(char *buf, char *homeDir) +{ // [HGM] file name relative to homeDir. (Taken out of SafeOptionsDialog, because it is generally useful) + char *result = NULL; + if ((isalpha(buf[0]) && buf[1] == ':') || + (buf[0] == '\\' && buf[1] == '\\')) { + return strdup(buf); + } else { + char buf2[MSG_SIZ], buf3[MSG_SIZ]; + char *dummy; + GetCurrentDirectory(MSG_SIZ, buf3); + SetCurrentDirectory(homeDir); + if (GetFullPathName(buf, MSG_SIZ, buf2, &dummy)) { + result = strdup(buf2); + } else { + result = strdup(buf); + } + SetCurrentDirectory(buf3); + } + return result; +} + /*---------------------------------------------------------------------------*\ * * General Options Dialog functions @@ -2494,7 +2516,7 @@ SaveOptionsDialog(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam) if (IsDlgButtonChecked(hDlg, OPT_Autosave)) { appData.autoSaveGames = TRUE; if (IsDlgButtonChecked(hDlg, OPT_AVPrompt)) { - appData.saveGameFile = ""; + ASSIGN(appData.saveGameFile, ""); // [HGM] make sure value is ALWAYS in allocated memory } else /*if (IsDlgButtonChecked(hDlg, OPT_AVToFile))*/ { GetDlgItemText(hDlg, OPT_AVFilename, buf, MSG_SIZ); if (*buf == NULLCHAR) { @@ -2502,25 +2524,12 @@ SaveOptionsDialog(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam) _("Option Error"), MB_OK|MB_ICONEXCLAMATION); return FALSE; } - if ((isalpha(buf[0]) && buf[1] == ':') || - (buf[0] == '\\' && buf[1] == '\\')) { - appData.saveGameFile = strdup(buf); - } else { - char buf2[MSG_SIZ], buf3[MSG_SIZ]; - char *dummy; - GetCurrentDirectory(MSG_SIZ, buf3); - SetCurrentDirectory(installDir); - if (GetFullPathName(buf, MSG_SIZ, buf2, &dummy)) { - appData.saveGameFile = strdup(buf2); - } else { - appData.saveGameFile = strdup(buf); - } - SetCurrentDirectory(buf3); - } + FREE(appData.saveGameFile); + appData.saveGameFile = InterpretFileName(buf, homeDir); } } else { appData.autoSaveGames = FALSE; - appData.saveGameFile = ""; + ASSIGN(appData.saveGameFile, ""); } appData.oldSaveStyle = IsDlgButtonChecked(hDlg, OPT_Old); appData.saveOutOfBookInfo = IsDlgButtonChecked( hDlg, OPT_OutOfBookInfo ); diff --git a/winboard/wsettings.c b/winboard/wsettings.c index db41147..7246590 100644 --- a/winboard/wsettings.c +++ b/winboard/wsettings.c @@ -352,8 +352,9 @@ GetOptionValues(HWND hDlg, ChessProgramState *cps, Option *optionList) if(!success) break; if(!cps) { char *p; - if(*(char**)optionList[j].target) free(*(char**)optionList[j].target); - *(char**)optionList[j].target = p = text; + p = (optionList[j].type != FileName ? strdup(text) : InterpretFileName(text, homeDir)); // all files relative to homeDir! + FREE(*(char**)optionList[j].target); *(char**)optionList[j].target = p; + free(text); text = p; while(*p++ = *text++) if(p[-1] == '\r') p--; // crush CR break; } -- 1.7.0.4