Plug memory leak, filenames relative to installDir
authorH.G. Muller <h.g.muller@hccnet.nl>
Fri, 6 May 2011 17:17:56 +0000 (19:17 +0200)
committerH.G. Muller <h.g.muller@hccnet.nl>
Sat, 7 May 2011 14:36:09 +0000 (16:36 +0200)
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
common.h
frontend.h
winboard/winboard.c
winboard/woptions.c
winboard/wsettings.c

diff --git a/args.h b/args.h
index 2471d13..c0e569b 100644 (file)
--- 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;
index 6140d5e..791e5e5 100644 (file)
--- 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
 
index 2263fde..f498bc4 100644 (file)
@@ -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));
index 67e950a..0ff5e0b 100644 (file)
@@ -156,6 +156,7 @@ char *programName;
 char *settingsFileName;\r
 Boolean saveSettingsOnExit;\r
 char installDir[MSG_SIZ];\r
+char homeDir[MSG_SIZ];\r
 int errorExitStatus;\r
 \r
 BoardSize boardSize;\r
@@ -989,6 +990,7 @@ InitInstance(HINSTANCE hInstance, int nCmdShow, LPSTR lpCmdLine)
   } else {\r
     GetCurrentDirectory(MSG_SIZ, installDir);\r
   }\r
+  safeStrCpy(homeDir, installDir, MSG_SIZ);\r
   gameInfo.boardWidth = gameInfo.boardHeight = 8; // [HGM] won't have open window otherwise\r
   screenWidth = screenHeight = 1000; // [HGM] placement: kludge to allow calling EnsureOnScreen from InitAppData\r
   InitAppData(lpCmdLine);      /* Get run-time parameters */\r
index 801fe04..9aa820f 100644 (file)
@@ -135,6 +135,28 @@ VOID SetLoadOptionEnables(HWND hDlg);
 VOID SetSaveOptionEnables(HWND hDlg);\r
 VOID SetTimeControlEnables(HWND hDlg);\r
 \r
+char *\r
+InterpretFileName(char *buf, char *homeDir)\r
+{ // [HGM] file name relative to homeDir. (Taken out of SafeOptionsDialog, because it is generally useful)\r
+  char *result = NULL;\r
+  if ((isalpha(buf[0]) && buf[1] == ':') ||\r
+    (buf[0] == '\\' && buf[1] == '\\')) {\r
+    return strdup(buf);\r
+  } else {\r
+    char buf2[MSG_SIZ], buf3[MSG_SIZ];\r
+    char *dummy;\r
+    GetCurrentDirectory(MSG_SIZ, buf3);\r
+    SetCurrentDirectory(homeDir);\r
+    if (GetFullPathName(buf, MSG_SIZ, buf2, &dummy)) {\r
+      result = strdup(buf2);\r
+    } else {\r
+      result = strdup(buf);\r
+    }\r
+    SetCurrentDirectory(buf3);\r
+  }\r
+  return result;\r
+}\r
+\r
 /*---------------------------------------------------------------------------*\\r
  *\r
  * General Options Dialog functions\r
@@ -2494,7 +2516,7 @@ SaveOptionsDialog(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam)
       if (IsDlgButtonChecked(hDlg, OPT_Autosave)) {\r
        appData.autoSaveGames = TRUE;\r
        if (IsDlgButtonChecked(hDlg, OPT_AVPrompt)) {\r
-         appData.saveGameFile = "";\r
+         ASSIGN(appData.saveGameFile, ""); // [HGM] make sure value is ALWAYS in allocated memory\r
        } else /*if (IsDlgButtonChecked(hDlg, OPT_AVToFile))*/ {\r
          GetDlgItemText(hDlg, OPT_AVFilename, buf, MSG_SIZ);\r
          if (*buf == NULLCHAR) {\r
@@ -2502,25 +2524,12 @@ SaveOptionsDialog(HWND hDlg, UINT message, WPARAM wParam, LPARAM lParam)
                       _("Option Error"), MB_OK|MB_ICONEXCLAMATION);\r
            return FALSE;\r
          }\r
-         if ((isalpha(buf[0]) && buf[1] == ':') ||\r
-           (buf[0] == '\\' && buf[1] == '\\')) {\r
-           appData.saveGameFile = strdup(buf);\r
-         } else {\r
-           char buf2[MSG_SIZ], buf3[MSG_SIZ];\r
-           char *dummy;\r
-           GetCurrentDirectory(MSG_SIZ, buf3);\r
-           SetCurrentDirectory(installDir);\r
-           if (GetFullPathName(buf, MSG_SIZ, buf2, &dummy)) {\r
-             appData.saveGameFile = strdup(buf2);\r
-           } else {\r
-             appData.saveGameFile = strdup(buf);\r
-           }\r
-           SetCurrentDirectory(buf3);\r
-         }\r
+         FREE(appData.saveGameFile);\r
+         appData.saveGameFile = InterpretFileName(buf, homeDir);\r
        }\r
       } else {\r
        appData.autoSaveGames = FALSE;\r
-       appData.saveGameFile = "";\r
+       ASSIGN(appData.saveGameFile, "");\r
       }\r
       appData.oldSaveStyle = IsDlgButtonChecked(hDlg, OPT_Old);\r
       appData.saveOutOfBookInfo = IsDlgButtonChecked( hDlg, OPT_OutOfBookInfo );\r
index db41147..7246590 100644 (file)
@@ -352,8 +352,9 @@ GetOptionValues(HWND hDlg, ChessProgramState *cps, Option *optionList)
                if(!success) break;\r
                if(!cps) {\r
                    char *p;\r
-                   if(*(char**)optionList[j].target) free(*(char**)optionList[j].target);\r
-                   *(char**)optionList[j].target = p = text;\r
+                   p = (optionList[j].type != FileName ? strdup(text) : InterpretFileName(text, homeDir)); // all files relative to homeDir!\r
+                   FREE(*(char**)optionList[j].target); *(char**)optionList[j].target = p;
+                   free(text); text = p;\r
                    while(*p++ = *text++) if(p[-1] == '\r') p--; // crush CR\r
                    break;\r
                }\r