[patch#3 2/8] Code cleanup: new path to VEC in utils.c

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

http://sourceware.org/ml/gdb-patches/2012-03/msg01018.html
On Thu, 29 Mar 2012 23:13:46 +0200, Doug Evans wrote:
> We need a general utility routine (in a general utility kinda place)
> that takes a path list and returns a vec.
> There's a lot of code that could use it.

here it is.


Thanks,
Jan


gdb/
2012-04-01  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Code cleanup.
	* charset.c (find_charset_names): Remove variables ix and elt.
	Use free_char_ptr_vec.
	* elfread.c (build_id_to_debug_filename): New variables debugdir_vec,
	back_to and ix.  Use dirnames_to_char_ptr_vec.  Remove variable
	debugdir_end.  New variable debugdir_len.
	* gdb_vecs.h (free_char_ptr_vec, make_cleanup_free_char_ptr_vec)
	(dirnames_to_char_ptr_vec_append, dirnames_to_char_ptr_vec): New
	declarations.
	* progspace.c (clear_program_space_solib_cache): Remove variables ix
	and elt.  Use free_char_ptr_vec.
	* source.c (add_path): Remove variables argv, arg and argv_index.
	New variables dir_vec, back_to, ix and name.
	Use dirnames_to_char_ptr_vec_append.  Use freeargv instead of
	make_cleanup_freeargv.  Remove variable separator.  Simplify the code
	no longer expecting DIRNAME_SEPARATOR.
	(openp): Remove variable p, p1 and len.  New variables dir_vec,
	back_to, ix and dir.  Use dirnames_to_char_ptr_vec.  Simplify the code
	no longer expecting DIRNAME_SEPARATOR.
	* symfile.c (find_separate_debug_file): New variables debugdir_vec,
	back_to and ix.  Use dirnames_to_char_ptr_vec.  Remove variable
	debugdir_end.
	* utils.c (free_char_ptr_vec, do_free_char_ptr_vec)
	(make_cleanup_free_char_ptr_vec, dirnames_to_char_ptr_vec_append)
	(dirnames_to_char_ptr_vec): New functions.

--- a/gdb/charset.c
+++ b/gdb/charset.c
@@ -909,11 +909,8 @@ find_charset_names (void)
   if (fail)
     {
       /* Some error occurred, so drop the vector.  */
-      int ix;
-      char *elt;
-      for (ix = 0; VEC_iterate (char_ptr, charsets, ix, elt); ++ix)
-	xfree (elt);
-      VEC_truncate (char_ptr, charsets, 0);
+      free_char_ptr_vec (charsets);
+      charsets = NULL;
     }
   else
     VEC_safe_push (char_ptr, charsets, NULL);
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1123,6 +1123,9 @@ static char *
 build_id_to_debug_filename (struct build_id *build_id)
 {
   char *link, *debugdir, *retval = NULL;
+  VEC (char_ptr) *debugdir_vec;
+  struct cleanup *back_to;
+  int ix;
 
   /* DEBUG_FILE_DIRECTORY/.build-id/ab/cdef */
   link = alloca (strlen (debug_file_directory) + (sizeof "/.build-id/" - 1) + 1
@@ -1131,22 +1134,18 @@ build_id_to_debug_filename (struct build_id *build_id)
   /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
      cause "/.build-id/..." lookups.  */
 
-  debugdir = debug_file_directory;
-  do
+  debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
+  back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
+
+  for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
     {
-      char *s, *debugdir_end;
+      size_t debugdir_len = strlen (debugdir);
       gdb_byte *data = build_id->data;
       size_t size = build_id->size;
+      char *s;
 
-      while (*debugdir == DIRNAME_SEPARATOR)
-	debugdir++;
-
-      debugdir_end = strchr (debugdir, DIRNAME_SEPARATOR);
-      if (debugdir_end == NULL)
-	debugdir_end = &debugdir[strlen (debugdir)];
-
-      memcpy (link, debugdir, debugdir_end - debugdir);
-      s = &link[debugdir_end - debugdir];
+      memcpy (link, debugdir, debugdir_len);
+      s = &link[debugdir_len];
       s += sprintf (s, "/.build-id/");
       if (size > 0)
 	{
@@ -1171,11 +1170,9 @@ build_id_to_debug_filename (struct build_id *build_id)
 
       if (retval != NULL)
 	break;
-
-      debugdir = debugdir_end;
     }
-  while (*debugdir != 0);
 
+  do_cleanups (back_to);
   return retval;
 }
 
--- a/gdb/gdb_vecs.h
+++ b/gdb/gdb_vecs.h
@@ -25,4 +25,16 @@
 
 DEF_VEC_P (char_ptr);
 
+/* From utils.c: */
+
+extern void free_char_ptr_vec (VEC (char_ptr) *char_ptr_vec);
+
+extern struct cleanup *
+  make_cleanup_free_char_ptr_vec (VEC (char_ptr) *char_ptr_vec);
+
+extern void dirnames_to_char_ptr_vec_append (VEC (char_ptr) **vecp,
+					     const char *dirnames);
+
+extern VEC (char_ptr) *dirnames_to_char_ptr_vec (const char *dirnames);
+
 #endif /* GDB_VECS_H */
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -509,13 +509,10 @@ switch_to_program_space_and_thread (struct program_space *pspace)
 void
 clear_program_space_solib_cache (struct program_space *pspace)
 {
-  int ix;
-  char *name;
-
   VEC_free (so_list_ptr, pspace->added_solibs);
-  for (ix = 0; VEC_iterate (char_ptr, pspace->deleted_solibs, ix, name); ++ix)
-    xfree (name);
-  VEC_free (char_ptr, pspace->deleted_solibs);
+
+  free_char_ptr_vec (pspace->deleted_solibs);
+  pspace->deleted_solibs = NULL;
 }
 
 
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -440,62 +440,40 @@ add_path (char *dirname, char **which_path, int parse_separators)
 {
   char *old = *which_path;
   int prefix = 0;
-  char **argv = NULL;
-  char *arg;
-  int argv_index = 0;
+  VEC (char_ptr) *dir_vec = NULL;
+  struct cleanup *back_to;
+  int ix;
+  char *name;
 
   if (dirname == 0)
     return;
 
   if (parse_separators)
     {
+      char **argv, **argvp;
+
       /* This will properly parse the space and tab separators
-	 and any quotes that may exist.  DIRNAME_SEPARATOR will
-	 be dealt with later.  */
+	 and any quotes that may exist.  */
       argv = gdb_buildargv (dirname);
-      make_cleanup_freeargv (argv);
 
-      arg = argv[0];
+      for (argvp = argv; *argvp; argvp++)
+	dirnames_to_char_ptr_vec_append (&dir_vec, *argvp);
+
+      freeargv (argv);
     }
   else
-    {
-      arg = xstrdup (dirname);
-      make_cleanup (xfree, arg);
-    }
+    VEC_safe_push (char_ptr, dir_vec, xstrdup (dirname));
+  back_to = make_cleanup_free_char_ptr_vec (dir_vec);
 
-  do
+  for (ix = 0; VEC_iterate (char_ptr, dir_vec, ix, name); ++ix)
     {
-      char *name = arg;
       char *p;
       struct stat st;
 
-      {
-	char *separator = NULL;
-
-	/* Spaces and tabs will have been removed by buildargv().
-	   The directories will there be split into a list but
-	   each entry may still contain DIRNAME_SEPARATOR.  */
-	if (parse_separators)
-	  separator = strchr (name, DIRNAME_SEPARATOR);
-
-	if (separator == 0)
-	  p = arg = name + strlen (name);
-	else
-	  {
-	    p = separator;
-	    arg = p + 1;
-	    while (*arg == DIRNAME_SEPARATOR)
-	      ++arg;
-	  }
-
-	/* If there are no more directories in this argument then start
-	   on the next argument next time round the loop (if any).  */
-	if (*arg == '\0')
-	  arg = parse_separators ? argv[++argv_index] : NULL;
-      }
-
-      /* name is the start of the directory.
-	 p is the separator (or null) following the end.  */
+      /* Spaces and tabs will have been removed by buildargv().
+         NAME is the start of the directory.
+	 P is the '\0' following the end.  */
+      p = name + strlen (name);
 
       while (!(IS_DIR_SEPARATOR (*name) && p <= name + 1)	/* "/" */
 #ifdef HAVE_DOS_BASED_FILE_SYSTEM
@@ -576,26 +554,17 @@ add_path (char *dirname, char **which_path, int parse_separators)
 	char tinybuf[2];
 
 	p = *which_path;
-	while (1)
+	/* FIXME: we should use realpath() or its work-alike
+	   before comparing.  Then all the code above which
+	   removes excess slashes and dots could simply go away.  */
+	if (!filename_cmp (p, name))
 	  {
-	    /* FIXME: we should use realpath() or its work-alike
-	       before comparing.  Then all the code above which
-	       removes excess slashes and dots could simply go away.  */
-	    if (!filename_ncmp (p, name, len)
-		&& (p[len] == '\0' || p[len] == DIRNAME_SEPARATOR))
-	      {
-		/* Found it in the search path, remove old copy.  */
-		if (p > *which_path)
-		  p--;		/* Back over leading separator.  */
-		if (prefix > p - *which_path)
-		  goto skip_dup;	/* Same dir twice in one cmd.  */
-		memmove (p, &p[len + 1], strlen (&p[len + 1]) + 1);	/* Copy from next \0 or  : */
-	      }
-	    p = strchr (p, DIRNAME_SEPARATOR);
-	    if (p != 0)
-	      ++p;
-	    else
-	      break;
+	    /* Found it in the search path, remove old copy.  */
+	    if (p > *which_path)
+	      p--;		/* Back over leading separator.  */
+	    if (prefix > p - *which_path)
+	      goto skip_dup;	/* Same dir twice in one cmd.  */
+	    memmove (p, &p[len + 1], strlen (&p[len + 1]) + 1);	/* Copy from next \0 or  : */
 	  }
 
 	tinybuf[0] = DIRNAME_SEPARATOR;
@@ -628,7 +597,8 @@ add_path (char *dirname, char **which_path, int parse_separators)
     skip_dup:
       ;
     }
-  while (arg != NULL);
+
+  do_cleanups (back_to);
 }
 
 
@@ -709,10 +679,11 @@ openp (const char *path, int opts, const char *string,
 {
   int fd;
   char *filename;
-  const char *p;
-  const char *p1;
-  int len;
   int alloclen;
+  VEC (char_ptr) *dir_vec;
+  struct cleanup *back_to;
+  int ix;
+  char *dir;
 
   /* The open syscall MODE parameter is not specified.  */
   gdb_assert ((mode & O_CREAT) == 0);
@@ -775,16 +746,15 @@ openp (const char *path, int opts, const char *string,
   alloclen = strlen (path) + strlen (string) + 2;
   filename = alloca (alloclen);
   fd = -1;
-  for (p = path; p; p = p1 ? p1 + 1 : 0)
+
+  dir_vec = dirnames_to_char_ptr_vec (path);
+  back_to = make_cleanup_free_char_ptr_vec (dir_vec);
+
+  for (ix = 0; VEC_iterate (char_ptr, dir_vec, ix, dir); ++ix)
     {
-      p1 = strchr (p, DIRNAME_SEPARATOR);
-      if (p1)
-	len = p1 - p;
-      else
-	len = strlen (p);
+      size_t len = strlen (dir);
 
-      if (len == 4 && p[0] == '$' && p[1] == 'c'
-	  && p[2] == 'w' && p[3] == 'd')
+      if (strcmp (dir, "$cwd") == 0)
 	{
 	  /* Name is $cwd -- insert current directory name instead.  */
 	  int newlen;
@@ -802,8 +772,7 @@ openp (const char *path, int opts, const char *string,
       else
 	{
 	  /* Normal file name in path -- just use it.  */
-	  strncpy (filename, p, len);
-	  filename[len] = 0;
+	  strcpy (filename, dir);
 
 	  /* Don't search $cdir.  It's also a magic path like $cwd, but we
 	     don't have enough information to expand it.  The user *could*
@@ -812,7 +781,7 @@ openp (const char *path, int opts, const char *string,
 	     contexts.  If the user really has '$cdir' one can use './$cdir'.
 	     We can get $cdir when loading scripts.  When loading source files
 	     $cdir must have already been expanded to the correct value.  */
-	  if (strcmp (filename, "$cdir") == 0)
+	  if (strcmp (dir, "$cdir") == 0)
 	    continue;
 	}
 
@@ -831,6 +800,8 @@ openp (const char *path, int opts, const char *string,
 	}
     }
 
+  do_cleanups (back_to);
+
 done:
   if (filename_opened)
     {
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1441,6 +1441,9 @@ find_separate_debug_file (const char *dir,
   char *debugdir;
   char *debugfile;
   int i;
+  VEC (char_ptr) *debugdir_vec;
+  struct cleanup *back_to;
+  int ix;
 
   /* Set I to max (strlen (canon_dir), strlen (dir)).  */
   i = strlen (dir);
@@ -1475,20 +1478,12 @@ find_separate_debug_file (const char *dir,
      Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
      cause "/..." lookups.  */
 
-  debugdir = debug_file_directory;
-  do
-    {
-      char *debugdir_end;
-
-      while (*debugdir == DIRNAME_SEPARATOR)
-	debugdir++;
-
-      debugdir_end = strchr (debugdir, DIRNAME_SEPARATOR);
-      if (debugdir_end == NULL)
-	debugdir_end = &debugdir[strlen (debugdir)];
+  debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
+  back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
 
-      memcpy (debugfile, debugdir, debugdir_end - debugdir);
-      debugfile[debugdir_end - debugdir] = 0;
+  for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
+    {
+      strcpy (debugfile, debugdir);
       strcat (debugfile, "/");
       strcat (debugfile, dir);
       strcat (debugfile, debuglink);
@@ -1503,8 +1498,7 @@ find_separate_debug_file (const char *dir,
 			    strlen (gdb_sysroot)) == 0
 	  && IS_DIR_SEPARATOR (canon_dir[strlen (gdb_sysroot)]))
 	{
-	  memcpy (debugfile, debugdir, debugdir_end - debugdir);
-	  debugfile[debugdir_end - debugdir] = 0;
+	  strcpy (debugfile, debugdir);
 	  strcat (debugfile, canon_dir + strlen (gdb_sysroot));
 	  strcat (debugfile, "/");
 	  strcat (debugfile, debuglink);
@@ -1512,11 +1506,9 @@ find_separate_debug_file (const char *dir,
 	  if (separate_debug_file_exists (debugfile, crc32, objfile))
 	    return debugfile;
 	}
-
-      debugdir = debugdir_end;
     }
-  while (*debugdir != 0);
 
+  do_cleanups (back_to);
   xfree (debugfile);
   return NULL;
 }
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3802,6 +3802,95 @@ producer_is_gcc_ge_4 (const char *producer)
   return minor;
 }
 
+/* Call xfree for each element of CHAR_PTR_VEC and final VEC_free for
+   CHAR_PTR_VEC itself.
+
+   You must not modify CHAR_PTR_VEC after it got registered with this function
+   by make_cleanup as the CHAR_PTR_VEC base address may change on its updates.
+   Contrary to VEC_free this function does not (cannot) clear the pointer.  */
+
+void
+free_char_ptr_vec (VEC (char_ptr) *char_ptr_vec)
+{
+  int ix;
+  char *name;
+
+  for (ix = 0; VEC_iterate (char_ptr, char_ptr_vec, ix, name); ++ix)
+    xfree (name);
+  VEC_free (char_ptr, char_ptr_vec);
+}
+
+/* Helper for make_cleanup_free_char_ptr_vec.  */
+
+static void
+do_free_char_ptr_vec (void *arg)
+{
+  VEC (char_ptr) *char_ptr_vec = arg;
+
+  free_char_ptr_vec (char_ptr_vec);
+}
+
+/* Make cleanup handler calling xfree for each element of CHAR_PTR_VEC and
+   final VEC_free for CHAR_PTR_VEC itself.
+
+   You must not modify CHAR_PTR_VEC after this cleanup registration as the
+   CHAR_PTR_VEC base address may change on its updates.  Contrary to VEC_free
+   this function does not (cannot) clear the pointer.  */
+
+struct cleanup *
+make_cleanup_free_char_ptr_vec (VEC (char_ptr) *char_ptr_vec)
+{
+  return make_cleanup (do_free_char_ptr_vec, char_ptr_vec);
+}
+
+/* Extended version of dirnames_to_char_ptr_vec - additionally if *VECP is
+   non-NULL the new list elements from DIRNAMES are appended to the existing
+   *VECP list of entries.  *VECP address will be updated by this call.  */
+
+void
+dirnames_to_char_ptr_vec_append (VEC (char_ptr) **vecp, const char *dirnames)
+{
+  do
+    {
+      size_t this_len;
+      char *next_dir, *this_dir;
+
+      next_dir = strchr (dirnames, DIRNAME_SEPARATOR);
+      if (next_dir == NULL)
+	this_len = strlen (dirnames);
+      else
+	{
+	  this_len = next_dir - dirnames;
+	  next_dir++;
+	}
+
+      this_dir = xmalloc (this_len + 1);
+      memcpy (this_dir, dirnames, this_len);
+      this_dir[this_len] = '\0';
+      VEC_safe_push (char_ptr, *vecp, this_dir);
+
+      dirnames = next_dir;
+    }
+  while (dirnames != NULL);
+}
+
+/* Split DIRNAMES by DIRNAME_SEPARATOR delimiter and return a list of all the
+   elements in their original order.  For empty string ("") DIRNAMES return
+   list of one empty string ("") element.
+   
+   You may modify the returned strings.
+   Read free_char_ptr_vec for its cleanup.  */
+
+VEC (char_ptr) *
+dirnames_to_char_ptr_vec (const char *dirnames)
+{
+  VEC (char_ptr) *retval = NULL;
+  
+  dirnames_to_char_ptr_vec_append (&retval, dirnames);
+
+  return retval;
+}
+
 #ifdef HAVE_WAITPID
 
 #ifdef SIGALRM