[RFA,1/2] Change target_read_string to use unique_xmalloc_ptr

Message ID 20180328042424.15276-2-tom@tromey.com
State New
Headers show
Series
  • more cleanup removal
Related show

Commit Message

Tom Tromey March 28, 2018, 4:24 a.m.
This changes the out parameter of target_read_string to be a
unique_xmalloc_ptr.  This avoids a cleanup and sets the stage for more
cleanup removals.

This patch also removes a seemingly needless alloca from
print_subexp_standard.

gdb/ChangeLog
2018-03-27  Tom Tromey  <tom@tromey.com>

	* windows-nat.c (handle_output_debug_string, handle_exception):
	Update.
	* target.h (target_read_string): Update.
	* target.c (target_read_string): Change "string" to
	unique_xmalloc_ptr.
	* solib-svr4.c (open_symbol_file_object, svr4_read_so_list):
	Update.
	* solib-frv.c (frv_current_sos): Update.
	* solib-dsbt.c (dsbt_current_sos): Update.
	* solib-darwin.c (darwin_current_sos): Update.
	* linux-thread-db.c (inferior_has_bug): Update.
	* expprint.c (print_subexp_standard) <case OP_OBJC_MSGCALL>:
	Update.  Remove alloca.
	* ada-lang.c (ada_main_name): Update.
---
 gdb/ChangeLog         | 17 +++++++++++++++++
 gdb/ada-lang.c        |  5 ++---
 gdb/expprint.c        |  9 +++------
 gdb/linux-thread-db.c |  7 +++----
 gdb/solib-darwin.c    |  5 ++---
 gdb/solib-dsbt.c      |  7 +++----
 gdb/solib-frv.c       |  8 ++++----
 gdb/solib-svr4.c      | 10 ++++------
 gdb/target.c          |  5 +++--
 gdb/target.h          |  3 ++-
 gdb/windows-nat.c     | 28 ++++++++++++----------------
 11 files changed, 55 insertions(+), 49 deletions(-)

-- 
2.13.6

Comments

Simon Marchi March 30, 2018, 5:27 a.m. | #1
On 2018-03-28 00:24, Tom Tromey wrote:
> This changes the out parameter of target_read_string to be a

> unique_xmalloc_ptr.  This avoids a cleanup and sets the stage for more

> cleanup removals.

> 

> This patch also removes a seemingly needless alloca from

> print_subexp_standard.


That looks fine to me.

Simon

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 505d059244..11939d7798 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -913,7 +913,7 @@  char *
 ada_main_name (void)
 {
   struct bound_minimal_symbol msym;
-  static char *main_program_name = NULL;
+  static gdb::unique_xmalloc_ptr<char> main_program_name;
 
   /* For Ada, the name of the main procedure is stored in a specific
      string constant, generated by the binder.  Look for that symbol,
@@ -931,13 +931,12 @@  ada_main_name (void)
       if (main_program_name_addr == 0)
         error (_("Invalid address for Ada main program name."));
 
-      xfree (main_program_name);
       target_read_string (main_program_name_addr, &main_program_name,
                           1024, &err_code);
 
       if (err_code != 0)
         return NULL;
-      return main_program_name;
+      return main_program_name.get ();
     }
 
   /* The main procedure doesn't seem to be in Ada.  */
diff --git a/gdb/expprint.c b/gdb/expprint.c
index 9d1884f290..c906904599 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -240,7 +240,7 @@  print_subexp_standard (struct expression *exp, int *pos,
 
     case OP_OBJC_MSGCALL:
       {			/* Objective C message (method) call.  */
-	char *selector;
+	gdb::unique_xmalloc_ptr<char> selector;
 
 	(*pos) += 3;
 	nargs = longest_to_int (exp->elts[pc + 2].longconst);
@@ -256,8 +256,7 @@  print_subexp_standard (struct expression *exp, int *pos,
 	  {
 	    char *s, *nextS;
 
-	    s = (char *) alloca (strlen (selector) + 1);
-	    strcpy (s, selector);
+	    s = selector.get ();
 	    for (tem = 0; tem < nargs; tem++)
 	      {
 		nextS = strchr (s, ':');
@@ -270,11 +269,9 @@  print_subexp_standard (struct expression *exp, int *pos,
 	  }
 	else
 	  {
-	    fprintf_unfiltered (stream, " %s", selector);
+	    fprintf_unfiltered (stream, " %s", selector.get ());
 	  }
 	fprintf_unfiltered (stream, "]");
-	/* "selector" was malloc'd by target_read_string.  Free it.  */
-	xfree (selector);
 	return;
       }
 
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index cfedd98bd8..08e3cfbc8b 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -413,7 +413,7 @@  inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min)
 {
   struct bound_minimal_symbol version_msym;
   CORE_ADDR version_addr;
-  char *version;
+  gdb::unique_xmalloc_ptr<char> version;
   int err, got, retval = 0;
 
   version_msym = lookup_minimal_symbol (ver_symbol, NULL, NULL);
@@ -422,15 +422,14 @@  inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min)
 
   version_addr = BMSYMBOL_VALUE_ADDRESS (version_msym);
   got = target_read_string (version_addr, &version, 32, &err);
-  if (err == 0 && memchr (version, 0, got) == &version[got -1])
+  if (err == 0 && memchr (version.get (), 0, got) == version.get () + got - 1)
     {
       int major, minor;
 
-      retval = (sscanf (version, "%d.%d", &major, &minor) == 2
+      retval = (sscanf (version.get (), "%d.%d", &major, &minor) == 2
 		&& (major < ver_major_min
 		    || (major == ver_major_min && minor < ver_minor_min)));
     }
-  xfree (version);
 
   return retval;
 }
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index cf15148c36..d66938fb90 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -261,7 +261,7 @@  darwin_current_sos (void)
       CORE_ADDR path_addr;
       struct mach_o_header_external hdr;
       unsigned long hdr_val;
-      char *file_path;
+      gdb::unique_xmalloc_ptr<char> file_path;
       int errcode;
       struct so_list *newobj;
       struct cleanup *old_chain;
@@ -299,10 +299,9 @@  darwin_current_sos (void)
       lm_info_darwin *li = new lm_info_darwin;
       newobj->lm_info = li;
 
-      strncpy (newobj->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
+      strncpy (newobj->so_name, file_path.get (), SO_NAME_MAX_PATH_SIZE - 1);
       newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       strcpy (newobj->so_original_name, newobj->so_name);
-      xfree (file_path);
       li->lm_addr = load_addr;
 
       if (head == NULL)
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index d038cccb2f..b3b66baffc 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -696,7 +696,7 @@  dsbt_current_sos (void)
       if (dsbt_index != 0)
 	{
 	  int errcode;
-	  char *name_buf;
+	  gdb::unique_xmalloc_ptr<char> name_buf;
 	  struct int_elf32_dsbt_loadmap *loadmap;
 	  struct so_list *sop;
 	  CORE_ADDR addr;
@@ -727,11 +727,10 @@  dsbt_current_sos (void)
 	    {
 	      if (solib_dsbt_debug)
 		fprintf_unfiltered (gdb_stdlog, "current_sos: name = %s\n",
-				    name_buf);
+				    name_buf.get ());
 
-	      strncpy (sop->so_name, name_buf, SO_NAME_MAX_PATH_SIZE - 1);
+	      strncpy (sop->so_name, name_buf.get (), SO_NAME_MAX_PATH_SIZE - 1);
 	      sop->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
-	      xfree (name_buf);
 	      strcpy (sop->so_original_name, sop->so_name);
 	    }
 
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 7929efd786..e772da6115 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -377,7 +377,7 @@  frv_current_sos (void)
       if (got_addr != mgot)
 	{
 	  int errcode;
-	  char *name_buf;
+	  gdb::unique_xmalloc_ptr<char> name_buf;
 	  struct int_elf32_fdpic_loadmap *loadmap;
 	  struct so_list *sop;
 	  CORE_ADDR addr;
@@ -409,16 +409,16 @@  frv_current_sos (void)
 
 	  if (solib_frv_debug)
 	    fprintf_unfiltered (gdb_stdlog, "current_sos: name = %s\n",
-	                        name_buf);
+	                        name_buf.get ());
 	  
 	  if (errcode != 0)
 	    warning (_("Can't read pathname for link map entry: %s."),
 		     safe_strerror (errcode));
 	  else
 	    {
-	      strncpy (sop->so_name, name_buf, SO_NAME_MAX_PATH_SIZE - 1);
+	      strncpy (sop->so_name, name_buf.get (),
+		       SO_NAME_MAX_PATH_SIZE - 1);
 	      sop->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
-	      xfree (name_buf);
 	      strcpy (sop->so_original_name, sop->so_name);
 	    }
 
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 29e74daa51..d8d047d394 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -990,7 +990,7 @@  static int
 open_symbol_file_object (int from_tty)
 {
   CORE_ADDR lm, l_name;
-  char *filename;
+  gdb::unique_xmalloc_ptr<char> filename;
   int errcode;
   struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
   struct type *ptr_type = builtin_type (target_gdbarch ())->builtin_data_ptr;
@@ -1040,7 +1040,6 @@  open_symbol_file_object (int from_tty)
 
   /* Now fetch the filename from target memory.  */
   target_read_string (l_name, &filename, SO_NAME_MAX_PATH_SIZE - 1, &errcode);
-  make_cleanup (xfree, filename);
 
   if (errcode)
     {
@@ -1051,7 +1050,7 @@  open_symbol_file_object (int from_tty)
     }
 
   /* Have a pathname: read the symbol file.  */
-  symbol_file_add_main (filename, add_flags);
+  symbol_file_add_main (filename.get (), add_flags);
 
   do_cleanups (cleanups);
   return 1;
@@ -1339,7 +1338,7 @@  svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
   for (; lm != 0; prev_lm = lm, lm = next_lm)
     {
       int errcode;
-      char *buffer;
+      gdb::unique_xmalloc_ptr<char> buffer;
 
       so_list_up newobj (XCNEW (struct so_list));
 
@@ -1387,10 +1386,9 @@  svr4_read_so_list (CORE_ADDR lm, CORE_ADDR prev_lm,
 	  continue;
 	}
 
-      strncpy (newobj->so_name, buffer, SO_NAME_MAX_PATH_SIZE - 1);
+      strncpy (newobj->so_name, buffer.get (), SO_NAME_MAX_PATH_SIZE - 1);
       newobj->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
       strcpy (newobj->so_original_name, newobj->so_name);
-      xfree (buffer);
 
       /* If this entry has no name, or its name matches the name
 	 for the main executable, don't include it in the list.  */
diff --git a/gdb/target.c b/gdb/target.c
index 84f5228919..b9b2e757dd 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -993,7 +993,8 @@  target_xfer_status_to_string (enum target_xfer_status status)
    read.  */
 
 int
-target_read_string (CORE_ADDR memaddr, char **string, int len, int *errnop)
+target_read_string (CORE_ADDR memaddr, gdb::unique_xmalloc_ptr<char> *string,
+		    int len, int *errnop)
 {
   int tlen, offset, i;
   gdb_byte buf[4];
@@ -1053,7 +1054,7 @@  target_read_string (CORE_ADDR memaddr, char **string, int len, int *errnop)
       nbytes_read += tlen;
     }
 done:
-  *string = buffer;
+  string->reset (buffer);
   if (errnop != NULL)
     *errnop = errcode;
   return nbytes_read;
diff --git a/gdb/target.h b/gdb/target.h
index bfe9b826b2..51ac884e01 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1430,7 +1430,8 @@  int target_supports_disable_randomization (void);
 #define target_can_run_breakpoint_commands() \
   (*current_target.to_can_run_breakpoint_commands) (&current_target)
 
-extern int target_read_string (CORE_ADDR, char **, int, int *);
+extern int target_read_string (CORE_ADDR, gdb::unique_xmalloc_ptr<char> *,
+			       int, int *);
 
 /* For target_read_memory see target/target.h.  */
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 430cc60993..95e3c5816b 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -883,25 +883,25 @@  signal_event_command (const char *args, int from_tty)
 static int
 handle_output_debug_string (struct target_waitstatus *ourstatus)
 {
-  char *s = NULL;
+  gdb::unique_xmalloc_ptr<char> s;
   int retval = 0;
 
   if (!target_read_string
 	((CORE_ADDR) (uintptr_t) current_event.u.DebugString.lpDebugStringData,
-	&s, 1024, 0)
-      || !s || !*s)
+	 &s, 1024, 0)
+      || !s || !*(s.get ()))
     /* nothing to do */;
-  else if (!startswith (s, _CYGWIN_SIGNAL_STRING))
+  else if (!startswith (s.get (), _CYGWIN_SIGNAL_STRING))
     {
 #ifdef __CYGWIN__
-      if (!startswith (s, "cYg"))
+      if (!startswith (s.get (), "cYg"))
 #endif
 	{
-	  char *p = strchr (s, '\0');
+	  char *p = strchr (s.get (), '\0');
 
-	  if (p > s && *--p == '\n')
+	  if (p > s.get () && *--p == '\n')
 	    *p = '\0';
-	  warning (("%s"), s);
+	  warning (("%s"), s.get ());
 	}
     }
 #ifdef __CYGWIN__
@@ -915,7 +915,7 @@  handle_output_debug_string (struct target_waitstatus *ourstatus)
 	 to be stored at the given address in the inferior.  Tell gdb
 	 to treat this like a real signal.  */
       char *p;
-      int sig = strtol (s + sizeof (_CYGWIN_SIGNAL_STRING) - 1, &p, 0);
+      int sig = strtol (s.get () + sizeof (_CYGWIN_SIGNAL_STRING) - 1, &p, 0);
       gdb_signal gotasig = gdb_signal_from_host (sig);
 
       ourstatus->value.sig = gotasig;
@@ -938,8 +938,6 @@  handle_output_debug_string (struct target_waitstatus *ourstatus)
     }
 #endif
 
-  if (s)
-    xfree (s);
   return retval;
 }
 
@@ -1193,18 +1191,16 @@  handle_exception (struct target_waitstatus *ourstatus)
 	  if (named_thread != NULL)
 	    {
 	      int thread_name_len;
-	      char *thread_name;
+	      gdb::unique_xmalloc_ptr<char> thread_name;
 
 	      thread_name_len = target_read_string (thread_name_target,
 						    &thread_name, 1025, NULL);
 	      if (thread_name_len > 0)
 		{
-		  thread_name[thread_name_len - 1] = '\0';
+		  thread_name.get ()[thread_name_len - 1] = '\0';
 		  xfree (named_thread->name);
-		  named_thread->name = thread_name;
+		  named_thread->name = thread_name.release ();
 		}
-	      else
-		xfree (thread_name);
 	    }
 	  ourstatus->value.sig = GDB_SIGNAL_TRAP;
 	  result = HANDLE_EXCEPTION_IGNORED;