On Nov 1, 2011 6:13 AM, <[email protected]> wrote: > > From: Paulo Zanoni <[email protected]> > > We call xf86penConfigDirFiles twice, so we overwrite the configDirPath > variable, losing the pointer. If we move the pointer management to the > upper layer (the function callers), they will be able to call these > functions as many times as they want, but they'll have to free those > returned values. > > 4,097 bytes in 1 blocks are definitely lost in loss record 625 of 632 > at 0x4C2779D: malloc (in vgpreload_memcheck-amd64-linux.so) > by 0x4D7899: DoSubstitution (scan.c:615) > by 0x4D87B0: OpenConfigDir (scan.c:845) > by 0x4D8A2D: xf86openConfigDirFiles (scan.c:955) > by 0x49031F: xf86HandleConfigFile (xf86Config.c:2327) > by 0x49A9BF: InitOutput (xf86Init.c:365) > by 0x425A7A: main (main.c:204) > > Signed-off-by: Paulo Zanoni <[email protected]> > --- > hw/xfree86/common/xf86Config.c | 6 +++++- > hw/xfree86/parser/scan.c | 22 ++++++++-------------- > hw/xfree86/parser/xf86Parser.h | 8 ++++---- > 3 files changed, 17 insertions(+), 19 deletions(-) > > diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c > index cb4be42..5ab3e67 100644 > --- a/hw/xfree86/common/xf86Config.c > +++ b/hw/xfree86/common/xf86Config.c > @@ -2301,7 +2301,7 @@ checkInput(serverLayoutPtr layout, Bool > implicit_layout) { > ConfigStatus > xf86HandleConfigFile(Bool autoconfig) > { > - const char *filename, *dirname, *sysdirname; > + char *filename, *dirname, *sysdirname; > char *filesearch, *dirsearch; > MessageType filefrom = X_DEFAULT; > MessageType dirfrom = X_DEFAULT; > @@ -2353,6 +2353,10 @@ xf86HandleConfigFile(Bool autoconfig) > return CONFIG_NOFILE; > } > > + free(filename); > + free(dirname); > + free(sysdirname); > + > if ((xf86configptr = xf86readConfigFile ()) == NULL) { > xf86Msg(X_ERROR, "Problem parsing the config file\n"); > return CONFIG_PARSE_ERROR; > diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c > index 99b3257..668237b 100644 > --- a/hw/xfree86/parser/scan.c > +++ b/hw/xfree86/parser/scan.c > @@ -101,8 +101,6 @@ static int builtinIndex = 0; > static int configPos = 0; /* current readers position */ > static int configLineNo = 0; /* linenumber */ > static char *configBuf, *configRBuf; /* buffer for lines */ > -static char *configPath; /* path to config file */ > -static char *configDirPath; /* path to config dir */ > static char *configSection = NULL; /* name of current section being > parsed */ > static int numFiles = 0; /* number of config files */ > static int curFileIndex = 0; /* index of current config file */ > @@ -892,7 +890,8 @@ xf86initConfigFiles(void) > * of the located files. > * > * The return value is a pointer to the actual name of the file that was > - * opened. When no file is found, the return value is NULL. > + * opened. When no file is found, the return value is NULL. The caller > should > + * free() the returned value. > * > * The escape sequences allowed in the search path are defined above. > * > @@ -914,7 +913,7 @@ xf86initConfigFiles(void) > "%P/lib/X11/%X" > #endif > > -const char * > +char * > xf86openConfigFile(const char *path, const char *cmdline, const char > *projroot) > { > if (!path || !path[0]) > @@ -923,8 +922,7 @@ xf86openConfigFile(const char *path, const char *cmdline, > const char *projroot) > projroot = PROJECTROOT; > > /* Search for a config file */ > - configPath = OpenConfigFile(path, cmdline, projroot, XCONFIGFILE); > - return configPath; > + return OpenConfigFile(path, cmdline, projroot, XCONFIGFILE); > } > > /* > @@ -937,12 +935,13 @@ xf86openConfigFile(const char *path, const char > *cmdline, const char *projroot) > * fails if it is not found. > * > * The return value is a pointer to the actual name of the direcoty that was > - * opened. When no directory is found, the return value is NULL. > + * opened. When no directory is found, the return value is NULL. The caller > + * should free() the returned value. > * > * The escape sequences allowed in the search path are defined above. > * > */ > -const char * > +char * > xf86openConfigDirFiles(const char *path, const char *cmdline, > const char *projroot) > { > @@ -952,8 +951,7 @@ xf86openConfigDirFiles(const char *path, const char > *cmdline, > projroot = PROJECTROOT; > > /* Search for the multiconf directory */ > - configDirPath = OpenConfigDir(path, cmdline, projroot, XCONFIGDIR); > - return configDirPath; > + return OpenConfigDir(path, cmdline, projroot, XCONFIGDIR); > } > > void > @@ -961,10 +959,6 @@ xf86closeConfigFile (void) > { > int i; > > - free (configPath); > - configPath = NULL; > - free (configDirPath); > - configDirPath = NULL; > free (configRBuf); > configRBuf = NULL; > free (configBuf); > diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h > index a8785c5..c31fdc4 100644 > --- a/hw/xfree86/parser/xf86Parser.h > +++ b/hw/xfree86/parser/xf86Parser.h > @@ -487,10 +487,10 @@ xf86ConfigSymTabRec, *xf86ConfigSymTabPtr; > * prototypes for public functions > */ > extern void xf86initConfigFiles(void); > -extern const char *xf86openConfigFile(const char *path, const char *cmdline, > - const char *projroot); > -extern const char *xf86openConfigDirFiles(const char *path, const char > *cmdline, > - const char *projroot); > +extern char *xf86openConfigFile(const char *path, const char *cmdline, > + const char *projroot); > +extern char *xf86openConfigDirFiles(const char *path, const char *cmdline, > + const char *projroot); > extern void xf86setBuiltinConfig(const char *config[]); > extern XF86ConfigPtr xf86readConfigFile(void); > extern void xf86closeConfigFile(void);
Seems pretty reasonable, but do you know why xf86openConfigDirFiles is called twice? Oh, I guess it's because we also look at the system config dir. Now I recall thinking this would leak but not really caring to fix it at the time. :) I certainly like the idea of passing around variables when possible and not poking globals. These functions also get called from hw/xwin/winconfig.c. I'm not sure how used that code is anymore, but I was trying to keep it updated at the time. Besides that, Reviewed-by: Dan Nicholson <[email protected]> _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
