fix memory leaks in prefix.c.

Zack Weinberg zack@wolery.cumb.org
Sun Mar 12 09:36:00 GMT 2000


This patch cleans up prefix.c.  The main purpose is to fix the memory
leaks.  update_path() is now guaranteed to return a malloced copy of
the string you passed in, possibly modified.  Before, there was no way
to tell if it had copied the string or not.  All the internal leaks
are also removed.  There's one exception - get_key_value will leak on
Win32 hosts, because the result of the registry lookup is in malloced
memory, but the result of getenv isn't.  I see no good way around this.

I also moved cpplib's simplify_path() function into prefix.c, and
caused update_path not to dink with the directory separators.
Instead, you can call simplify_path on the result if you want.

I haven't tested this in a context where update_path is actually
expected to change pathnames.  It does, however, complete a bootstrap
successfully in the normal environment.

Why isn't prefix.c part of libiberty?

OK to commit?

zw

	* prefix.c (save_string): Inline into sole caller, set_std_prefix.
	(lookup_key): Inline into sole caller, get_key_value.
	(concat): Delete.
	(get_key_value): Use alloca to create <key>_ROOT envariable
	names.
	(translate_name): Takes three arguments.  Do all recursive
	lookup in here.  Take care not to leak memory.
	(update_path): Pass code, key, and path to translate_name
	separately. If we don't need to translate the path, xstrdup
	it.  Don't change directory separators here.
	(simplify_path): Moved here from cppfiles.c.  Use
	DIR_SEPARATOR and DIR_SEPARATOR_2.
	* prefix.h: Prototype simplify_path.

	* cppfiles.c (_cpp_simplify_pathname): Moved to prefix.c and
	renamed simplify_path.
	* cppinit.c (initialize_standard_includes): No need to strdup
	the result of update_path.
	* gcc.c (add_prefix): Likewise.  Call simplify_path on result
	of update_path.
	
===================================================================
Index: prefix.c
--- prefix.c	2000/02/13 19:59:29	1.24
+++ prefix.c	2000/03/12 17:25:56
@@ -60,8 +60,7 @@ Boston, MA 02111-1307, USA.  */
    Once all this is done, any '/' will be converted to DIR_SEPARATOR,
    if they are different. 
 
-   NOTE:  using resolve_keyed_path under Win32 requires linking with
-   advapi32.dll.  */
+   NOTE: under Win32, this code requires linking with advapi32.dll.  */
 
 
 #include "config.h"
@@ -72,120 +71,40 @@ Boston, MA 02111-1307, USA.  */
 #include "prefix.h"
 
 static const char *std_prefix = PREFIX;
+static size_t std_prefix_len = sizeof PREFIX - 1;
 
-static const char *get_key_value	PARAMS ((char *));
-static const char *translate_name	PARAMS ((const char *));
-static char *save_string		PARAMS ((const char *, int));
+static const char *get_key_value	PARAMS ((const char *));
+static char *translate_name		PARAMS ((int, const char *,
+						 const char *));
 
 #if defined(_WIN32) && defined(ENABLE_WIN32_REGISTRY)
-static char *lookup_key		PARAMS ((char *));
 static HKEY reg_key = (HKEY) INVALID_HANDLE_VALUE;
 #endif
 
-/* Given KEY, as above, return its value.  */
-
-static const char *
-get_key_value (key)
-     char *key;
-{
-  const char *prefix = 0;
-  char *temp = 0;
-
-#if defined(_WIN32) && defined(ENABLE_WIN32_REGISTRY)
-  prefix = lookup_key (key);
-#endif
-
-  if (prefix == 0)
-    prefix = getenv (temp = concat (key, "_ROOT", NULL_PTR));
-
-  if (prefix == 0)
-    prefix = std_prefix;
-
-  if (temp)
-    free (temp);
-
-  return prefix;
-}
-
-/* Concatenate a sequence of strings, returning the result.
-
-   This function is based on the one in libiberty.  */
-
-char *
-concat VPARAMS ((const char *first, ...))
+/* Reset the standard prefix */
+void
+set_std_prefix (prefix, len)
+  const char *prefix;
+  int len;
 {
-  register int length;
-  register char *newstr;
-  register char *end;
-  register const char *arg;
-  va_list args;
-#ifndef ANSI_PROTOTYPES
-  const char *first;
-#endif
-
-  /* First compute the size of the result and get sufficient memory.  */
-
-  VA_START (args, first);
-#ifndef ANSI_PROTOTYPES
-  first = va_arg (args, const char *);
-#endif
+  char *new = xmalloc (len + 1);
 
-  arg = first;
-  length = 0;
+  memcpy (new, prefix, len);
+  new[len] = '\0';
 
-  while (arg != 0)
-    {
-      length += strlen (arg);
-      arg = va_arg (args, const char *);
-    }
-
-  newstr = (char *) xmalloc (length + 1);
-  va_end (args);
-
-  /* Now copy the individual pieces to the result string.  */
-
-  VA_START (args, first);
-#ifndef ANSI_PROTOTYPES
-  first = va_arg (args, char *);
-#endif
-
-  end = newstr;
-  arg = first;
-  while (arg != 0)
-    {
-      while (*arg)
-	*end++ = *arg++;
-      arg = va_arg (args, const char *);
-    }
-  *end = '\000';
-  va_end (args);
-
-  return (newstr);
+  std_prefix = new;
+  std_prefix_len = len;
 }
 
-/* Return a copy of a string that has been placed in the heap.  */
+/* Given KEY, as above, return its value.  */
 
-static char *
-save_string (s, len)
-  const char *s;
-  int len;
+static const char *
+get_key_value (key)
+     const char *key;
 {
-  register char *result = xmalloc (len + 1);
-
-  bcopy (s, result, len);
-  result[len] = 0;
-  return result;
-}
+  const char *prefix = 0;
 
 #if defined(_WIN32) && defined(ENABLE_WIN32_REGISTRY)
-
-/* Look up "key" in the registry, as above.  */
-
-static char *
-lookup_key (key)
-     char *key;
-{
-  char *dst;
   DWORD size;
   DWORD type;
   LONG res;
@@ -211,73 +130,95 @@ lookup_key (key)
     }
 
   size = 32;
-  dst = (char *) xmalloc (size);
+  prefix = xmalloc (size);
 
-  res = RegQueryValueExA (reg_key, key, 0, &type, dst, &size);
+  res = RegQueryValueExA (reg_key, key, 0, &type, (char *)prefix, &size);
   if (res == ERROR_MORE_DATA && type == REG_SZ)
     {
-      dst = (char *) xrealloc (dst, size);
-      res = RegQueryValueExA (reg_key, key, 0, &type, dst, &size);
+      prefix = xrealloc (prefix, size);
+      res = RegQueryValueExA (reg_key, key, 0, &type, (char *)prefix, &size);
     }
 
   if (type != REG_SZ || res != ERROR_SUCCESS)
     {
-      free (dst);
-      dst = 0;
+      free (prefix);
+      prefix = 0;
     }
-
-  return dst;
-}
 #endif
-
-/* If NAME starts with a '@' or '$', apply the translation rules above
-   and return a new name.  Otherwise, return the given name.  */
 
-static const char *
-translate_name (name)
-  const char *name;
-{
-  char code = name[0];
-  char *key;
-  const char *prefix = 0;
-  int keylen;
+  if (prefix == 0)
+    {
+      char *temp = alloca (strlen (key) + sizeof "_ROOT");
+      strcpy (temp, key);
+      strcat (temp, "_ROOT");
+      prefix = getenv (temp);
+    }
 
-  if (code != '@' && code != '$')
-    return name;
+  if (prefix == 0)
+    prefix = std_prefix;
 
-  for (keylen = 0;
-       (name[keylen + 1] != 0 && !IS_DIR_SEPARATOR (name[keylen + 1]));
-       keylen++)
-    ;
+  return prefix;
+}
 
-  key = (char *) alloca (keylen + 1);
-  strncpy (key, &name[1], keylen);
-  key[keylen] = 0;
+/* Look up KEY according to the rules given above (possibly recursively)
+   and prepend the final value to PATH.  */
 
-  name = &name[keylen + 1];
+static char *
+translate_name (code, key, path)
+     int code;
+     const char *key;
+     const char *path;
+{
+  const char *value;
+  char *result;
+  char *nkey = 0, *trailing = 0;
+  size_t keylen, vallen, traillen = 1;
 
-  if (code == '@')
+  for (;;)
     {
-      prefix = get_key_value (key);
-      if (prefix == 0)
-	prefix = std_prefix;
+      if (code == '@')
+	value = get_key_value (key);
+      else
+	value = getenv (key);
+      if (value == 0)
+	value = std_prefix;
+
+      if (value[0] != '@' && value[0] != '$')
+	break;
+
+      /* Have to do recursive lookup.  From value[1] to the first
+	 DIR_SEPARATOR (or end of string) is the new key, and the
+	 remainder is to be prepended literally.  */
+      code = value[0];
+      key = value + 1;
+      keylen = 0;
+      while (key[keylen] != '\0' && !IS_DIR_SEPARATOR (key[keylen]))
+	keylen++;
+      if (key[keylen] != '\0')
+	{
+	  nkey = xrealloc (nkey, keylen + 1);
+	  memcpy (nkey, key, keylen);
+	  nkey[keylen] = '\0';
+	  key = nkey;
+
+	  value += keylen + 1;
+	  vallen = strlen (value);
+	  trailing = xrealloc (trailing, traillen + vallen);
+	  memmove (trailing + vallen, trailing, traillen);
+	  memcpy (trailing, value, vallen);
+	  trailing[traillen + vallen] = '\0';
+	  traillen += vallen;
+	}
     }
-  else
-    prefix = getenv (key);
-
-  if (prefix == 0)
-    prefix = PREFIX;
 
-  /* Remove any trailing directory separator from what we got. First check
-     for an empty prefix.  */
-  if (prefix[0] && IS_DIR_SEPARATOR (prefix[strlen (prefix) - 1]))
-    {
-      char * temp = xstrdup (prefix);
-      temp[strlen (temp) - 1] = 0;
-      prefix = temp;
-    }
+  if (trailing)
+    result = concat (value, trailing, path, 0);
+  else
+    result = concat (value, path, 0);
 
-  return concat (prefix, name, NULL_PTR);
+  free (trailing);
+  free (nkey);
+  return result;
 }
 
 /* Update PATH using KEY if PATH starts with PREFIX.  */
@@ -287,50 +228,156 @@ update_path (path, key)
   const char *path;
   const char *key;
 {
-  if (! strncmp (path, std_prefix, strlen (std_prefix)) && key != 0)
+  char *npath;
+
+  if (key && !strncmp (path, std_prefix, std_prefix_len))
     {
-      if (key[0] != '$')
-	key = concat ("@", key, NULL_PTR);
+      int code;
 
-      path = concat (key, &path[strlen (std_prefix)], NULL_PTR);
+      code = key[0];
+      if (code == '$' || code == '@')
+	key++;
+      else
+	code = '@';
 
-      while (path[0] == '@' || path[0] == '$')
-	path = translate_name (path);
+      npath = translate_name (code, key, path + std_prefix_len);
     }
+  else
+    npath = xstrdup (path);
 
-#ifdef DIR_SEPARATOR_2
-  /* Convert DIR_SEPARATOR_2 to DIR_SEPARATOR. */
-  if (DIR_SEPARATOR != DIR_SEPARATOR_2)
-    {
-      char *new_path = xstrdup (path);
-      path = new_path;
-      do {
-	if (*new_path == DIR_SEPARATOR_2)
-	  *new_path = DIR_SEPARATOR;
-      } while (*new_path++);
-    }
+  return npath;
+}
+
+/* Simplify a path name in place, deleting redundant components.  This
+   reduces OS overhead and guarantees that equivalent paths compare
+   the same (modulo symlinks).
+
+   Transforms made:
+   foo/bar/../quux	foo/quux
+   foo/./bar		foo/bar
+   foo//bar		foo/bar
+   /../quux		/quux
+   //quux		//quux  (POSIX allows leading // as a namespace escape)
+
+   Guarantees no trailing slashes. All transforms reduce the length
+   of the string.
+ */
+void
+simplify_path (path)
+    char *path;
+{
+    char *from, *to;
+    char *base;
+    int absolute = 0;
+
+#if defined DIR_SEPARATOR_2 && DIR_SEPARATOR_2 != DIR_SEPARATOR
+    /* Convert all backslashes to slashes. */
+    for (from = path; *from; from++)
+	if (*from == DIR_SEPARATOR_2) *from = DIR_SEPARATOR;
 #endif
-      
-#if defined (DIR_SEPARATOR) && !defined (DIR_SEPARATOR_2)
-  if (DIR_SEPARATOR != '/')
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+    /* Skip over leading drive letter if present. */
+    if (ISALPHA (path[0]) && path[1] == ':')
+	from = to = &path[2];
+    else
+	from = to = path;
+#else
+    from = to = path;
+#endif
+    
+    /* Remove redundant initial /s.  */
+    if (*from == DIR_SEPARATOR)
     {
-      char *new_path = xstrdup (path);
-      path = new_path;
-      do {
-	if (*new_path == '/')
-	  *new_path = DIR_SEPARATOR;
-      } while (*new_path++);
+	absolute = 1;
+	to++;
+	from++;
+	if (*from == DIR_SEPARATOR)
+	{
+	    if (*++from == DIR_SEPARATOR)
+		/* 3 or more initial /s are equivalent to 1 /.  */
+		while (*++from == DIR_SEPARATOR);
+	    else
+		/* On some hosts // differs from /; Posix allows this.  */
+		to++;
+	}
     }
-#endif
+    base = to;
+    
+    for (;;)
+    {
+	while (*from == DIR_SEPARATOR)
+	    from++;
 
-  return path;
-}
+	if (from[0] == '.' && from[1] == DIR_SEPARATOR)
+	    from += 2;
+	else if (from[0] == '.' && from[1] == '\0')
+	    goto done;
+	else if (from[0] == '.' && from[1] == '.' && from[2] == DIR_SEPARATOR)
+	{
+	    if (base == to)
+	    {
+		if (absolute)
+		    from += 3;
+		else
+		{
+		    *to++ = *from++;
+		    *to++ = *from++;
+		    *to++ = *from++;
+		    base = to;
+		}
+	    }
+	    else
+	    {
+		to -= 2;
+		while (to > base && *to != DIR_SEPARATOR) to--;
+		if (*to == DIR_SEPARATOR)
+		    to++;
+		from += 3;
+	    }
+	}
+	else if (from[0] == '.' && from[1] == '.' && from[2] == '\0')
+	{
+	    if (base == to)
+	    {
+		if (!absolute)
+		{
+		    *to++ = *from++;
+		    *to++ = *from++;
+		}
+	    }
+	    else
+	    {
+		to -= 2;
+		while (to > base && *to != DIR_SEPARATOR) to--;
+		if (*to == DIR_SEPARATOR)
+		    to++;
+	    }
+	    goto done;
+	}
+	else
+	    /* Copy this component and trailing /, if any.  */
+	    while ((*to++ = *from++) != DIR_SEPARATOR)
+	    {
+		if (!to[-1])
+		{
+		    to--;
+		    goto done;
+		}
+	    }
+	
+    }
+    
+ done:
+    /* Trim trailing slash */
+    if (to[0] == DIR_SEPARATOR && (!absolute || to > path+1))
+	to--;
+
+    /* Change the empty string to "." so that stat() on the result
+       will always work. */
+    if (to == path)
+      *to++ = '.';
+    
+    *to = '\0';
 
-/* Reset the standard prefix */
-void
-set_std_prefix (prefix, len)
-  const char *prefix;
-  int len;
-{
-  std_prefix = save_string (prefix, len);
+    return;
 }
===================================================================
Index: prefix.h
--- prefix.h	1999/01/06 21:31:04	1.3
+++ prefix.h	2000/03/12 17:25:56
@@ -23,6 +23,7 @@ Boston, MA 02111-1307, USA.  */
 #define __GCC_PREFIX_H__
 
 extern const char *update_path PARAMS ((const char *, const char *));
+extern void simplify_path PARAMS ((char *));
 extern void set_std_prefix PARAMS ((const char *, int));
 
 #endif /* ! __GCC_PREFIX_H__ */
===================================================================
Index: cppfiles.c
--- cppfiles.c	2000/03/11 00:49:44	1.44
+++ cppfiles.c	2000/03/12 17:25:54
@@ -249,7 +249,7 @@ _cpp_find_include_file (pfile, fname, se
       memcpy (name, l->name, l->nlen);
       name[l->nlen] = '/';
       strcpy (&name[l->nlen+1], fname);
-      _cpp_simplify_pathname (name);
+      simplify_path (name);
       if (CPP_OPTIONS (pfile)->remap)
 	name = remap_filename (pfile, name, l);
 
@@ -1097,138 +1097,6 @@ init_input_buffer (pfile, fd, st)
   pfile->input_buffer_len = pipe_buf;
 }
 
-/* Simplify a path name in place, deleting redundant components.  This
-   reduces OS overhead and guarantees that equivalent paths compare
-   the same (modulo symlinks).
-
-   Transforms made:
-   foo/bar/../quux	foo/quux
-   foo/./bar		foo/bar
-   foo//bar		foo/bar
-   /../quux		/quux
-   //quux		//quux  (POSIX allows leading // as a namespace escape)
-
-   Guarantees no trailing slashes. All transforms reduce the length
-   of the string.
- */
-void
-_cpp_simplify_pathname (path)
-    char *path;
-{
-    char *from, *to;
-    char *base;
-    int absolute = 0;
-
-#if defined (HAVE_DOS_BASED_FILE_SYSTEM)
-    /* Convert all backslashes to slashes. */
-    for (from = path; *from; from++)
-	if (*from == '\\') *from = '/';
-    
-    /* Skip over leading drive letter if present. */
-    if (ISALPHA (path[0]) && path[1] == ':')
-	from = to = &path[2];
-    else
-	from = to = path;
-#else
-    from = to = path;
-#endif
-    
-    /* Remove redundant initial /s.  */
-    if (*from == '/')
-    {
-	absolute = 1;
-	to++;
-	from++;
-	if (*from == '/')
-	{
-	    if (*++from == '/')
-		/* 3 or more initial /s are equivalent to 1 /.  */
-		while (*++from == '/');
-	    else
-		/* On some hosts // differs from /; Posix allows this.  */
-		to++;
-	}
-    }
-    base = to;
-    
-    for (;;)
-    {
-	while (*from == '/')
-	    from++;
-
-	if (from[0] == '.' && from[1] == '/')
-	    from += 2;
-	else if (from[0] == '.' && from[1] == '\0')
-	    goto done;
-	else if (from[0] == '.' && from[1] == '.' && from[2] == '/')
-	{
-	    if (base == to)
-	    {
-		if (absolute)
-		    from += 3;
-		else
-		{
-		    *to++ = *from++;
-		    *to++ = *from++;
-		    *to++ = *from++;
-		    base = to;
-		}
-	    }
-	    else
-	    {
-		to -= 2;
-		while (to > base && *to != '/') to--;
-		if (*to == '/')
-		    to++;
-		from += 3;
-	    }
-	}
-	else if (from[0] == '.' && from[1] == '.' && from[2] == '\0')
-	{
-	    if (base == to)
-	    {
-		if (!absolute)
-		{
-		    *to++ = *from++;
-		    *to++ = *from++;
-		}
-	    }
-	    else
-	    {
-		to -= 2;
-		while (to > base && *to != '/') to--;
-		if (*to == '/')
-		    to++;
-	    }
-	    goto done;
-	}
-	else
-	    /* Copy this component and trailing /, if any.  */
-	    while ((*to++ = *from++) != '/')
-	    {
-		if (!to[-1])
-		{
-		    to--;
-		    goto done;
-		}
-	    }
-	
-    }
-    
- done:
-    /* Trim trailing slash */
-    if (to[0] == '/' && (!absolute || to > path+1))
-	to--;
-
-    /* Change the empty string to "." so that stat() on the result
-       will always work. */
-    if (to == path)
-      *to++ = '.';
-    
-    *to = '\0';
-
-    return;
-}
 
 /* It is not clear when this should be used if at all, so I've
    disabled it until someone who understands VMS can look at it. */
===================================================================
Index: cpphash.h
--- cpphash.h	2000/03/08 23:35:18	1.20
+++ cpphash.h	2000/03/12 17:25:54
@@ -262,7 +262,6 @@ extern void _cpp_dump_definition  PARAMS
 					   DEFINITION *));
 
 /* In cppfiles.c */
-extern void _cpp_simplify_pathname	PARAMS ((char *));
 extern int _cpp_find_include_file	PARAMS ((cpp_reader *, const char *,
 						struct file_name_list *,
 						IHASH **, int *));
===================================================================
Index: cppinit.c
--- cppinit.c	2000/03/12 04:33:26	1.60
+++ cppinit.c	2000/03/12 17:25:55
@@ -335,7 +335,7 @@ append_include_chain (pfile, pend, dir, 
   struct stat st;
   unsigned int len;
 
-  _cpp_simplify_pathname (dir);
+  simplify_path (dir);
   if (stat (dir, &st))
     {
       /* Dirs that don't exist are silently ignored. */
@@ -826,8 +826,7 @@ initialize_standard_includes (pfile)
 	  || (opts->cplusplus
 	      && !opts->no_standard_cplusplus_includes))
 	{
-	  /* XXX Potential memory leak! */
-	  char *str = xstrdup (update_path (p->fname, p->component));
+	  char *str = update_path (p->fname, p->component);
 	  append_include_chain (pfile, opts->pending, str, SYSTEM,
 				p->cxx_aware);
 	}
===================================================================
Index: gcc.c
--- gcc.c	2000/03/06 18:05:51	1.133
+++ gcc.c	2000/03/12 17:25:56
@@ -2394,12 +2394,13 @@ add_prefix (pprefix, prefix, component, 
   /* Keep track of the longest prefix */
 
   prefix = update_path (prefix, component);
+  simplify_path (prefix);
   len = strlen (prefix);
   if (len > pprefix->max_len)
     pprefix->max_len = len;
 
   pl = (struct prefix_list *) xmalloc (sizeof (struct prefix_list));
-  pl->prefix = save_string (prefix, len);
+  pl->prefix = prefix;
   pl->require_machine_suffix = require_machine_suffix;
   pl->used_flag_ptr = warn;
   if (warn)


More information about the Gcc-patches mailing list