Patchwork [RFA,3/4] Change openp et al to use a unique_xmalloc_ptr

login
register
mail settings
Submitter Tom Tromey
Date Feb. 13, 2018, 11:39 p.m.
Message ID <20180213233907.11259-4-tom@tromey.com>
Download mbox | patch
Permalink /patch/25932/
State New
Headers show

Comments

Tom Tromey - Feb. 13, 2018, 11:39 p.m.
This changes openp, source_full_path_of, and find_and_open_source to
take a unique_xmalloc_ptr, rather than a char*, as an outgoing
argument type.  This simplifies the API, ownership-wise, and allows
for the removal of some cleanups.

2018-02-13  Tom Tromey  <tom@tromey.com>

	* symfile.c (symfile_bfd_open): Update.
	* source.h (openp, source_full_path_of, find_and_open_source):
	Change argument type to unique_xmalloc_ptr.
	* source.c (openp): Take a unique_xmalloc_ptr.
	(source_full_path_of, find_and_open_source): Likewise.
	(open_source_file, symtab_to_fullname): Update.
	* solist.h (struct target_so_ops) <find_and_open_solib>: Take a
	unique_xmalloc_ptr.
	* solib.c (solib_find_1): Use unique_xmalloc_ptr.
	(exec_file_find): Update.
	* psymtab.c (psymtab_to_fullname): Update.
	* nto-tdep.h (nto_find_and_open_solib): Update.
	* nto-tdep.c (nto_find_and_open_solib): Change temp_path to a
	unique_xmalloc_ptr.
	* exec.c (exec_file_attach): Update.
	* dwarf2read.c (try_open_dwop_file): Use unique_xmalloc_ptr.
	* cli/cli-cmds.c (find_and_open_script): Use unique_xmalloc_ptr.
---
 gdb/ChangeLog      | 20 +++++++++++++++++++
 gdb/cli/cli-cmds.c |  5 ++---
 gdb/dwarf2read.c   | 23 +++++++++++++---------
 gdb/exec.c         |  9 +++------
 gdb/nto-tdep.c     |  6 +++---
 gdb/nto-tdep.h     |  3 ++-
 gdb/psymtab.c      |  6 +++---
 gdb/solib.c        | 56 +++++++++++++++++++++++-------------------------------
 gdb/solist.h       |  3 ++-
 gdb/source.c       | 41 ++++++++++++++++++---------------------
 gdb/source.h       |  7 ++++---
 gdb/symfile.c      |  9 ++-------
 12 files changed, 98 insertions(+), 90 deletions(-)

Patch

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index c3962e939e..95bd58ce54 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -506,10 +506,9 @@  find_and_open_script (const char *script_file, int search_path)
 
   /* Search for and open 'file' on the search path used for source
      files.  Put the full location in *FULL_PATHP.  */
-  char *temp_path;
+  gdb::unique_xmalloc_ptr<char> full_path;
   fd = openp (source_path, search_flags,
-	      file.get (), O_RDONLY, &temp_path);
-  gdb::unique_xmalloc_ptr<char> full_path (temp_path);
+	      file.get (), O_RDONLY, &full_path);
 
   if (fd == -1)
     return opened;
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 93453431c1..d6d3d4a20f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -12881,35 +12881,40 @@  try_open_dwop_file (struct dwarf2_per_objfile *dwarf2_per_objfile,
 		    const char *file_name, int is_dwp, int search_cwd)
 {
   int desc;
-  char *absolute_name;
   /* Blech.  OPF_TRY_CWD_FIRST also disables searching the path list if
      FILE_NAME contains a '/'.  So we can't use it.  Instead prepend "."
      to debug_file_directory.  */
-  char *search_path;
+  const char *search_path;
   static const char dirname_separator_string[] = { DIRNAME_SEPARATOR, '\0' };
 
+  gdb::unique_xmalloc_ptr<char> search_path_holder;
   if (search_cwd)
     {
       if (*debug_file_directory != '\0')
-	search_path = concat (".", dirname_separator_string,
-			      debug_file_directory, (char *) NULL);
+	{
+	  search_path_holder.reset (concat (".", dirname_separator_string,
+					    debug_file_directory,
+					    (char *) NULL));
+	  search_path = search_path_holder.get ();
+	}
       else
-	search_path = xstrdup (".");
+	search_path = ".";
     }
   else
-    search_path = xstrdup (debug_file_directory);
+    search_path = debug_file_directory;
 
   openp_flags flags = OPF_RETURN_REALPATH;
   if (is_dwp)
     flags |= OPF_SEARCH_IN_PATH;
+
+  gdb::unique_xmalloc_ptr<char> absolute_name;
   desc = openp (search_path, flags, file_name,
 		O_RDONLY | O_BINARY, &absolute_name);
-  xfree (search_path);
   if (desc < 0)
     return NULL;
 
-  gdb_bfd_ref_ptr sym_bfd (gdb_bfd_open (absolute_name, gnutarget, desc));
-  xfree (absolute_name);
+  gdb_bfd_ref_ptr sym_bfd (gdb_bfd_open (absolute_name.get (),
+					 gnutarget, desc));
   if (sym_bfd == NULL)
     return NULL;
   bfd_set_cacheable (sym_bfd.get (), 1);
diff --git a/gdb/exec.c b/gdb/exec.c
index 79baf9c5fb..0b4237ff63 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -290,12 +290,10 @@  exec_file_attach (const char *filename, int from_tty)
 	}
       else
 	{
-	  char *temp_pathname;
-
 	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST,
 				filename, write_files ?
 				O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
-				&temp_pathname);
+				&scratch_storage);
 #if defined(__GO32__) || defined(_WIN32) || defined(__CYGWIN__)
 	  if (scratch_chan < 0)
 	    {
@@ -306,14 +304,13 @@  exec_file_attach (const char *filename, int from_tty)
 				    exename, write_files ?
 				    O_RDWR | O_BINARY
 				    : O_RDONLY | O_BINARY,
-				    &temp_pathname);
+				    &scratch_storage);
 	    }
 #endif
 	  if (scratch_chan < 0)
 	    perror_with_name (filename);
 
-	  scratch_storage.reset (temp_pathname);
-	  scratch_pathname = temp_pathname;
+	  scratch_pathname = scratch_storage.get ();
 
 	  /* gdb_bfd_open (and its variants) prefers canonicalized
 	     pathname for better BFD caching.  */
diff --git a/gdb/nto-tdep.c b/gdb/nto-tdep.c
index 30db99c759..8eb864b871 100644
--- a/gdb/nto-tdep.c
+++ b/gdb/nto-tdep.c
@@ -89,7 +89,7 @@  nto_map_arch_to_cputype (const char *arch)
 
 int
 nto_find_and_open_solib (const char *solib, unsigned o_flags,
-			 char **temp_pathname)
+			 gdb::unique_xmalloc_ptr<char> *temp_pathname)
 {
   char *buf, *arch_path, *nto_root;
   const char *endian;
@@ -143,9 +143,9 @@  nto_find_and_open_solib (const char *solib, unsigned o_flags,
       if (temp_pathname)
 	{
 	  if (ret >= 0)
-	    *temp_pathname = gdb_realpath (arch_path).release ();
+	    *temp_pathname = gdb_realpath (arch_path);
 	  else
-	    *temp_pathname = NULL;
+	    temp_pathname->reset (NULL);
 	}
     }
   return ret;
diff --git a/gdb/nto-tdep.h b/gdb/nto-tdep.h
index 4eaea48c5d..f82cad1306 100644
--- a/gdb/nto-tdep.h
+++ b/gdb/nto-tdep.h
@@ -170,7 +170,8 @@  void nto_relocate_section_addresses (struct so_list *,
 
 int nto_map_arch_to_cputype (const char *);
 
-int nto_find_and_open_solib (const char *, unsigned, char **);
+int nto_find_and_open_solib (const char *, unsigned,
+			     gdb::unique_xmalloc_ptr<char> *);
 
 enum gdb_osabi nto_elf_osabi_sniffer (bfd *abfd);
 
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index dc13fd560d..ac0ee0a5a6 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1203,14 +1203,14 @@  psymtab_to_fullname (struct partial_symtab *ps)
      to handle cases like the file being moved.  */
   if (ps->fullname == NULL)
     {
-      int fd = find_and_open_source (ps->filename, ps->dirname, &ps->fullname);
+      gdb::unique_xmalloc_ptr<char> fullname;
+      int fd = find_and_open_source (ps->filename, ps->dirname, &fullname);
+      ps->fullname = fullname.release ();
 
       if (fd >= 0)
 	close (fd);
       else
 	{
-	  gdb::unique_xmalloc_ptr<char> fullname;
-
 	  /* rewrite_source_path would be applied by find_and_open_source, we
 	     should report the pathname where GDB tried to find the file.  */
 
diff --git a/gdb/solib.c b/gdb/solib.c
index c732df6150..c1bb227d3f 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -154,7 +154,7 @@  solib_find_1 (const char *in_pathname, int *fd, int is_solib)
 {
   const struct target_so_ops *ops = solib_ops (target_gdbarch ());
   int found_file = -1;
-  char *temp_pathname = NULL;
+  gdb::unique_xmalloc_ptr<char> temp_pathname;
   const char *fskind = effective_target_file_system_kind ();
   const char *sysroot = gdb_sysroot;
   int prefix_len, orig_prefix_len;
@@ -216,7 +216,7 @@  solib_find_1 (const char *in_pathname, int *fd, int is_solib)
   */
 
   if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
-    temp_pathname = xstrdup (in_pathname);
+    temp_pathname.reset (xstrdup (in_pathname));
   else
     {
       int need_dir_separator;
@@ -242,23 +242,21 @@  solib_find_1 (const char *in_pathname, int *fd, int is_solib)
 			     || strcmp (TARGET_SYSROOT_PREFIX, sysroot) == 0);
 
       /* Cat the prefixed pathname together.  */
-      temp_pathname = concat (sysroot,
-			      need_dir_separator ? SLASH_STRING : "",
-			      in_pathname, (char *) NULL);
+      temp_pathname.reset (concat (sysroot,
+				   need_dir_separator ? SLASH_STRING : "",
+				   in_pathname, (char *) NULL));
     }
 
   /* Handle files to be accessed via the target.  */
-  if (is_target_filename (temp_pathname))
+  if (is_target_filename (temp_pathname.get ()))
     {
       if (fd != NULL)
 	*fd = -1;
-      return gdb::unique_xmalloc_ptr<char> (temp_pathname);
+      return temp_pathname;
     }
 
   /* Now see if we can open it.  */
-  found_file = gdb_open_cloexec (temp_pathname, O_RDONLY | O_BINARY, 0);
-  if (found_file < 0)
-    xfree (temp_pathname);
+  found_file = gdb_open_cloexec (temp_pathname.get (), O_RDONLY | O_BINARY, 0);
 
   /* If the search in gdb_sysroot failed, and the path name has a
      drive spec (e.g, c:/foo), try stripping ':' from the drive spec,
@@ -270,33 +268,30 @@  solib_find_1 (const char *in_pathname, int *fd, int is_solib)
       && HAS_TARGET_DRIVE_SPEC (fskind, in_pathname))
     {
       int need_dir_separator = !IS_DIR_SEPARATOR (in_pathname[2]);
-      char *drive = savestring (in_pathname, 1);
+      char drive[2] = { in_pathname[0], '\0' };
 
-      temp_pathname = concat (sysroot,
-			      SLASH_STRING,
-			      drive,
-			      need_dir_separator ? SLASH_STRING : "",
-			      in_pathname + 2, (char *) NULL);
-      xfree (drive);
+      temp_pathname.reset (concat (sysroot,
+				   SLASH_STRING,
+				   drive,
+				   need_dir_separator ? SLASH_STRING : "",
+				   in_pathname + 2, (char *) NULL));
 
-      found_file = gdb_open_cloexec (temp_pathname, O_RDONLY | O_BINARY, 0);
+      found_file = gdb_open_cloexec (temp_pathname.get (),
+				     O_RDONLY | O_BINARY, 0);
       if (found_file < 0)
 	{
-	  xfree (temp_pathname);
-
 	  /* If the search in gdb_sysroot still failed, try fully
 	     stripping the drive spec, and trying once more in the
 	     sysroot before giving up.
 
 	     c:/foo/bar.dll ==> /sysroot/foo/bar.dll.  */
 
-	  temp_pathname = concat (sysroot,
-				  need_dir_separator ? SLASH_STRING : "",
-				  in_pathname + 2, (char *) NULL);
+	  temp_pathname.reset (concat (sysroot,
+				       need_dir_separator ? SLASH_STRING : "",
+				       in_pathname + 2, (char *) NULL));
 
-	  found_file = gdb_open_cloexec (temp_pathname, O_RDONLY | O_BINARY, 0);
-	  if (found_file < 0)
-	    xfree (temp_pathname);
+	  found_file = gdb_open_cloexec (temp_pathname.get (),
+					 O_RDONLY | O_BINARY, 0);
 	}
     }
 
@@ -306,7 +301,7 @@  solib_find_1 (const char *in_pathname, int *fd, int is_solib)
      needs to be freed.  */
 
   if (found_file < 0)
-    temp_pathname = NULL;
+    temp_pathname.reset (NULL);
 
   /* If the search in gdb_sysroot failed, and the path name is
      absolute at this point, make it relative.  (openp will try and open the
@@ -368,7 +363,7 @@  solib_find_1 (const char *in_pathname, int *fd, int is_solib)
   else
     *fd = found_file;
 
-  return gdb::unique_xmalloc_ptr<char> (temp_pathname);
+  return temp_pathname;
 }
 
 /* Return the full pathname of the main executable, or NULL if not
@@ -410,10 +405,7 @@  exec_file_find (const char *in_pathname, int *fd)
 	 (If that fails, we'll just fall back on the original
 	 filename.  Not much more we can do...)  */
 
-      char *full_path = NULL;
-      if (source_full_path_of (in_pathname, &full_path))
-	result.reset (full_path);
-      else
+      if (!source_full_path_of (in_pathname, &result))
 	result.reset (xstrdup (in_pathname));
       if (fd != NULL)
 	*fd = -1;
diff --git a/gdb/solist.h b/gdb/solist.h
index aba00ebb21..1df45e4d93 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -136,7 +136,8 @@  struct target_so_ops
        pointer to a malloc'd and realpath'd copy of SONAME is stored there,
        otherwise NULL is stored there.  */
     int (*find_and_open_solib) (const char *soname,
-        unsigned o_flags, char **temp_pathname);
+				unsigned o_flags,
+				gdb::unique_xmalloc_ptr<char> *temp_pathname);
 
     /* Hook for looking up global symbols in a library-specific way.  */
     struct block_symbol (*lookup_lib_global_symbol)
diff --git a/gdb/source.c b/gdb/source.c
index eba06f6e6b..8a27b2e666 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -737,7 +737,7 @@  is_regular_file (const char *name, int *errno_ptr)
     >>>>  eg executable, non-directory.  */
 int
 openp (const char *path, openp_flags opts, const char *string,
-       int mode, char **filename_opened)
+       int mode, gdb::unique_xmalloc_ptr<char> *filename_opened)
 {
   int fd;
   char *filename;
@@ -896,11 +896,11 @@  done:
     {
       /* If a file was opened, canonicalize its filename.  */
       if (fd < 0)
-	*filename_opened = NULL;
+	filename_opened->reset (NULL);
       else if ((opts & OPF_RETURN_REALPATH) != 0)
-	*filename_opened = gdb_realpath (filename).release ();
+	*filename_opened = gdb_realpath (filename);
       else
-	*filename_opened = gdb_abspath (filename).release ();
+	*filename_opened = gdb_abspath (filename);
     }
 
   errno = last_errno;
@@ -920,7 +920,8 @@  done:
 
    Else, this functions returns 0, and FULL_PATHNAME is set to NULL.  */
 int
-source_full_path_of (const char *filename, char **full_pathname)
+source_full_path_of (const char *filename,
+		     gdb::unique_xmalloc_ptr<char> *full_pathname)
 {
   int fd;
 
@@ -929,7 +930,7 @@  source_full_path_of (const char *filename, char **full_pathname)
 	      filename, O_RDONLY, full_pathname);
   if (fd < 0)
     {
-      *full_pathname = NULL;
+      full_pathname->reset (NULL);
       return 0;
     }
 
@@ -1011,7 +1012,7 @@  rewrite_source_path (const char *path)
 int
 find_and_open_source (const char *filename,
 		      const char *dirname,
-		      char **fullname)
+		      gdb::unique_xmalloc_ptr<char> *fullname)
 {
   char *path = source_path;
   const char *p;
@@ -1024,27 +1025,21 @@  find_and_open_source (const char *filename,
       /* The user may have requested that source paths be rewritten
          according to substitution rules he provided.  If a substitution
          rule applies to this path, then apply it.  */
-      char *rewritten_fullname = rewrite_source_path (*fullname).release ();
+      gdb::unique_xmalloc_ptr<char> rewritten_fullname
+	= rewrite_source_path (fullname->get ());
 
       if (rewritten_fullname != NULL)
-        {
-          xfree (*fullname);
-          *fullname = rewritten_fullname;
-        }
+	*fullname = std::move (rewritten_fullname);
 
-      result = gdb_open_cloexec (*fullname, OPEN_MODE, 0);
+      result = gdb_open_cloexec (fullname->get (), OPEN_MODE, 0);
       if (result >= 0)
 	{
-	  char *lpath = gdb_realpath (*fullname).release ();
-
-	  xfree (*fullname);
-	  *fullname = lpath;
+	  *fullname = gdb_realpath (fullname->get ());
 	  return result;
 	}
 
       /* Didn't work -- free old one, try again.  */
-      xfree (*fullname);
-      *fullname = NULL;
+      fullname->reset (NULL);
     }
 
   gdb::unique_xmalloc_ptr<char> rewritten_dirname;
@@ -1115,7 +1110,10 @@  open_source_file (struct symtab *s)
   if (!s)
     return -1;
 
-  return find_and_open_source (s->filename, SYMTAB_DIRNAME (s), &s->fullname);
+  gdb::unique_xmalloc_ptr<char> fullname;
+  int fd = find_and_open_source (s->filename, SYMTAB_DIRNAME (s), &fullname);
+  s->fullname = fullname.release ();
+  return fd;
 }
 
 /* Finds the fullname that a symtab represents.
@@ -1135,8 +1133,7 @@  symtab_to_fullname (struct symtab *s)
      to handle cases like the file being moved.  */
   if (s->fullname == NULL)
     {
-      int fd = find_and_open_source (s->filename, SYMTAB_DIRNAME (s),
-				     &s->fullname);
+      int fd = open_source_file (s);
 
       if (fd >= 0)
 	close (fd);
diff --git a/gdb/source.h b/gdb/source.h
index a21e919b28..a8769506a0 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -32,9 +32,10 @@  enum openp_flag
 
 DEF_ENUM_FLAGS_TYPE(openp_flag, openp_flags);
 
-extern int openp (const char *, openp_flags, const char *, int, char **);
+extern int openp (const char *, openp_flags, const char *, int,
+		  gdb::unique_xmalloc_ptr<char> *);
 
-extern int source_full_path_of (const char *, char **);
+extern int source_full_path_of (const char *, gdb::unique_xmalloc_ptr<char> *);
 
 extern void mod_path (const char *, char **);
 
@@ -67,7 +68,7 @@  extern void init_source_path (void);
      FULLNAME is set to NULL.  */
 extern int find_and_open_source (const char *filename,
 				 const char *dirname,
-				 char **fullname);
+				 gdb::unique_xmalloc_ptr<char> *fullname);
 
 /* Open a source file given a symtab S.  Returns a file descriptor or
    negative number for error.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index ab6ec1cdad..699d9e6fe0 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1719,12 +1719,10 @@  gdb_bfd_ref_ptr
 symfile_bfd_open (const char *name)
 {
   int desc = -1;
-  struct cleanup *back_to = make_cleanup (null_cleanup, 0);
 
+  gdb::unique_xmalloc_ptr<char> absolute_name;
   if (!is_target_filename (name))
     {
-      char *absolute_name;
-
       gdb::unique_xmalloc_ptr<char> expanded_name (tilde_expand (name));
 
       /* Look down path for it, allocate 2nd new malloc'd copy.  */
@@ -1745,8 +1743,7 @@  symfile_bfd_open (const char *name)
       if (desc < 0)
 	perror_with_name (expanded_name.get ());
 
-      make_cleanup (xfree, absolute_name);
-      name = absolute_name;
+      name = absolute_name.get ();
     }
 
   gdb_bfd_ref_ptr sym_bfd (gdb_bfd_open (name, gnutarget, desc));
@@ -1761,8 +1758,6 @@  symfile_bfd_open (const char *name)
     error (_("`%s': can't read symbols: %s."), name,
 	   bfd_errmsg (bfd_get_error ()));
 
-  do_cleanups (back_to);
-
   return sym_bfd;
 }