This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA 15/23] Use std::vector to avoid cleanups


This patch introduces std::vector in several spots in gdb that were
using xmalloc and a cleanup.

The simple_search_memory change warrants a bit of extra attention.  It
was previously using malloc to try to handle memory allocation
failures.  I don't know whether this was actually important.

2017-05-02  Tom Tromey  <tom@tromey.com>

	* valops.c (search_struct_method): Use std::vector.
	* valarith.c (value_concat): Use std::vector.
	* target.c (memory_xfer_partial): Use std::vector.
	(simple_search_memory): Likewise.
	* printcmd.c (find_string_backward): Use std::vector.
	* mi/mi-main.c (mi_cmd_data_write_memory): Use std::vector.
	* gcore.c (gcore_copy_callback): Use std::vector.
	* elfread.c (elf_rel_plt_read): Use std::vector.
	* cp-valprint.c (cp_print_value): Use std::vector.
	* cli/cli-dump.c (restore_section_callback): Use std::vector.
---
 gdb/ChangeLog      | 13 +++++++++++++
 gdb/cli/cli-dump.c | 11 +++--------
 gdb/cp-valprint.c  | 11 +++--------
 gdb/elfread.c      | 22 +++++++---------------
 gdb/gcore.c        | 12 ++++--------
 gdb/mi/mi-main.c   | 11 +++--------
 gdb/printcmd.c     | 10 +++-------
 gdb/target.c       | 36 ++++++++++--------------------------
 gdb/valarith.c     | 28 ++++++++++------------------
 gdb/valops.c       | 10 +++-------
 10 files changed, 59 insertions(+), 105 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 03e733b..9f75d1e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 2017-05-02  Tom Tromey  <tom@tromey.com>
 
+	* valops.c (search_struct_method): Use std::vector.
+	* valarith.c (value_concat): Use std::vector.
+	* target.c (memory_xfer_partial): Use std::vector.
+	(simple_search_memory): Likewise.
+	* printcmd.c (find_string_backward): Use std::vector.
+	* mi/mi-main.c (mi_cmd_data_write_memory): Use std::vector.
+	* gcore.c (gcore_copy_callback): Use std::vector.
+	* elfread.c (elf_rel_plt_read): Use std::vector.
+	* cp-valprint.c (cp_print_value): Use std::vector.
+	* cli/cli-dump.c (restore_section_callback): Use std::vector.
+
+2017-05-02  Tom Tromey  <tom@tromey.com>
+
 	* jit.c (jit_reader_load_command): Use unique_xmalloc_ptr.
 
 2017-05-02  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 613ac1e..dfe99cd 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -434,8 +434,6 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
   bfd_vma sec_end    = sec_start + size;
   bfd_size_type sec_offset = 0;
   bfd_size_type sec_load_count = size;
-  struct cleanup *old_chain;
-  gdb_byte *buf;
   int ret;
 
   /* Ignore non-loadable sections, eg. from elf files.  */
@@ -463,9 +461,8 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
     sec_load_count -= sec_end - data->load_end;
 
   /* Get the data.  */
-  buf = (gdb_byte *) xmalloc (size);
-  old_chain = make_cleanup (xfree, buf);
-  if (!bfd_get_section_contents (ibfd, isec, buf, 0, size))
+  std::vector<gdb_byte> buf (size);
+  if (!bfd_get_section_contents (ibfd, isec, buf.data (), 0, size))
     error (_("Failed to read bfd file %s: '%s'."), bfd_get_filename (ibfd), 
 	   bfd_errmsg (bfd_get_error ()));
 
@@ -487,11 +484,9 @@ restore_section_callback (bfd *ibfd, asection *isec, void *args)
 
   /* Write the data.  */
   ret = target_write_memory (sec_start + sec_offset + data->load_offset, 
-			     buf + sec_offset, sec_load_count);
+			     &buf[sec_offset], sec_load_count);
   if (ret != 0)
     warning (_("restore: memory write failed (%s)."), safe_strerror (ret));
-  do_cleanups (old_chain);
-  return;
 }
 
 static void
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 73fe03d..61507bd 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -534,23 +534,18 @@ cp_print_value (struct type *type, struct type *real_type,
 	      if ((boffset + offset) < 0
 		  || (boffset + offset) >= TYPE_LENGTH (real_type))
 		{
-		  gdb_byte *buf;
-		  struct cleanup *back_to;
+		  std::vector<gdb_byte> buf (TYPE_LENGTH (baseclass));
 
-		  buf = (gdb_byte *) xmalloc (TYPE_LENGTH (baseclass));
-		  back_to = make_cleanup (xfree, buf);
-
-		  if (target_read_memory (address + boffset, buf,
+		  if (target_read_memory (address + boffset, buf.data (),
 					  TYPE_LENGTH (baseclass)) != 0)
 		    skip = 1;
 		  base_val = value_from_contents_and_address (baseclass,
-							      buf,
+							      buf.data (),
 							      address + boffset);
 		  baseclass = value_type (base_val);
 		  thisoffset = 0;
 		  boffset = 0;
 		  thistype = baseclass;
-		  do_cleanups (back_to);
 		}
 	      else
 		{
diff --git a/gdb/elfread.c b/gdb/elfread.c
index fba2026..a292fa8 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -540,9 +540,6 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
   asection *plt, *relplt, *got_plt;
   int plt_elf_idx;
   bfd_size_type reloc_count, reloc;
-  char *string_buffer = NULL;
-  size_t string_buffer_size = 0;
-  struct cleanup *back_to;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
   size_t ptr_size = TYPE_LENGTH (ptr_type);
@@ -576,7 +573,7 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
   if (! bed->s->slurp_reloc_table (obfd, relplt, dyn_symbol_table, TRUE))
     return;
 
-  back_to = make_cleanup (free_current_contents, &string_buffer);
+  std::vector<char> string_buffer;
 
   reloc_count = relplt->size / elf_section_data (relplt)->this_hdr.sh_entsize;
   for (reloc = 0; reloc < reloc_count; reloc++)
@@ -584,6 +581,7 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
       const char *name;
       struct minimal_symbol *msym;
       CORE_ADDR address;
+      const char *got_suffix = SYMBOL_GOT_PLT_SUFFIX;
       const size_t got_suffix_len = strlen (SYMBOL_GOT_PLT_SUFFIX);
       size_t name_len;
 
@@ -601,24 +599,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,
 	 OBJFILE the symbol is undefined and the objfile having NAME defined
 	 may not yet have been loaded.  */
 
-      if (string_buffer_size < name_len + got_suffix_len + 1)
-	{
-	  string_buffer_size = 2 * (name_len + got_suffix_len);
-	  string_buffer = (char *) xrealloc (string_buffer, string_buffer_size);
-	}
-      memcpy (string_buffer, name, name_len);
-      memcpy (&string_buffer[name_len], SYMBOL_GOT_PLT_SUFFIX,
-	      got_suffix_len + 1);
+      string_buffer.clear ();
+      string_buffer.insert (string_buffer.end (), name, name + name_len);
+      string_buffer.insert (string_buffer.end (), got_suffix,
+			    got_suffix + got_suffix_len + 1);
 
-      msym = record_minimal_symbol (reader, string_buffer,
+      msym = record_minimal_symbol (reader, string_buffer.data (),
 				    name_len + got_suffix_len,
                                     true, address, mst_slot_got_plt, got_plt,
 				    objfile);
       if (msym)
 	SET_MSYMBOL_SIZE (msym, ptr_size);
     }
-
-  do_cleanups (back_to);
 }
 
 /* The data pointer is htab_t for gnu_ifunc_record_cache_unchecked.  */
diff --git a/gdb/gcore.c b/gdb/gcore.c
index c32d2ff..d8cbbdc 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -548,8 +548,6 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
 {
   bfd_size_type size, total_size = bfd_section_size (obfd, osec);
   file_ptr offset = 0;
-  struct cleanup *old_chain = NULL;
-  gdb_byte *memhunk;
 
   /* Read-only sections are marked; we don't have to copy their contents.  */
   if ((bfd_get_section_flags (obfd, osec) & SEC_LOAD) == 0)
@@ -560,8 +558,7 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
     return;
 
   size = std::min (total_size, (bfd_size_type) MAX_COPY_BYTES);
-  memhunk = (gdb_byte *) xmalloc (size);
-  old_chain = make_cleanup (xfree, memhunk);
+  std::vector<gdb_byte> memhunk (size);
 
   while (total_size > 0)
     {
@@ -569,7 +566,7 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
 	size = total_size;
 
       if (target_read_memory (bfd_section_vma (obfd, osec) + offset,
-			      memhunk, size) != 0)
+			      memhunk.data (), size) != 0)
 	{
 	  warning (_("Memory read failed for corefile "
 		     "section, %s bytes at %s."),
@@ -577,7 +574,8 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
 		   paddress (target_gdbarch (), bfd_section_vma (obfd, osec)));
 	  break;
 	}
-      if (!bfd_set_section_contents (obfd, osec, memhunk, offset, size))
+      if (!bfd_set_section_contents (obfd, osec, memhunk.data (),
+				     offset, size))
 	{
 	  warning (_("Failed to write corefile contents (%s)."),
 		   bfd_errmsg (bfd_get_error ()));
@@ -587,8 +585,6 @@ gcore_copy_callback (bfd *obfd, asection *osec, void *ignored)
       total_size -= size;
       offset += size;
     }
-
-  do_cleanups (old_chain);	/* Frees MEMHUNK.  */
 }
 
 static int
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index aa44876..f4adda6 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1658,8 +1658,6 @@ mi_cmd_data_write_memory (const char *command, char **argv, int argc)
   /* FIXME: ezannoni 2000-02-17 LONGEST could possibly not be big
      enough when using a compiler other than GCC.  */
   LONGEST value;
-  gdb_byte *buffer;
-  struct cleanup *old_chain;
   long offset = 0;
   int oind = 0;
   char *oarg;
@@ -1706,13 +1704,10 @@ mi_cmd_data_write_memory (const char *command, char **argv, int argc)
   /* Get the value as a number.  */
   value = parse_and_eval_address (argv[3]);
   /* Get the value into an array.  */
-  buffer = (gdb_byte *) xmalloc (word_size);
-  old_chain = make_cleanup (xfree, buffer);
-  store_signed_integer (buffer, word_size, byte_order, value);
+  std::vector<gdb_byte> buffer (word_size);
+  store_signed_integer (buffer.data (), word_size, byte_order, value);
   /* Write it down to memory.  */
-  write_memory_with_notification (addr, buffer, word_size);
-  /* Free the buffer.  */
-  do_cleanups (old_chain);
+  write_memory_with_notification (addr, buffer.data (), word_size);
 }
 
 /* Implementation of the -data-write-memory-bytes command.
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 02d6e1c..e72f02c 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -948,8 +948,6 @@ find_string_backward (struct gdbarch *gdbarch,
                       int *strings_counted)
 {
   const int chunk_size = 0x20;
-  gdb_byte *buffer = NULL;
-  struct cleanup *cleanup = NULL;
   int read_error = 0;
   int chars_read = 0;
   int chars_to_read = chunk_size;
@@ -958,14 +956,13 @@ find_string_backward (struct gdbarch *gdbarch,
   CORE_ADDR string_start_addr = addr;
 
   gdb_assert (char_size == 1 || char_size == 2 || char_size == 4);
-  buffer = (gdb_byte *) xmalloc (chars_to_read * char_size);
-  cleanup = make_cleanup (xfree, buffer);
+  std::vector<gdb_byte> buffer (chars_to_read * char_size);
   while (count > 0 && read_error == 0)
     {
       int i;
 
       addr -= chars_to_read * char_size;
-      chars_read = read_memory_backward (gdbarch, addr, buffer,
+      chars_read = read_memory_backward (gdbarch, addr, buffer.data (),
                                          chars_to_read * char_size);
       chars_read /= char_size;
       read_error = (chars_read == chars_to_read) ? 0 : 1;
@@ -974,7 +971,7 @@ find_string_backward (struct gdbarch *gdbarch,
         {
           int offset = (chars_to_read - i - 1) * char_size;
 
-          if (integer_is_zero (buffer + offset, char_size)
+          if (integer_is_zero (&buffer[offset], char_size)
               || chars_counted == options->print_max)
             {
               /* Found '\0' or reached print_max.  As OFFSET is the offset to
@@ -997,7 +994,6 @@ find_string_backward (struct gdbarch *gdbarch,
       string_start_addr -= chars_counted * char_size;
     }
 
-  do_cleanups (cleanup);
   return string_start_addr;
 }
 
diff --git a/gdb/target.c b/gdb/target.c
index e526bcc..660c2ef 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1284,9 +1284,6 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
     }
   else
     {
-      gdb_byte *buf;
-      struct cleanup *old_chain;
-
       /* A large write request is likely to be partially satisfied
 	 by memory_xfer_partial_1.  We will continually malloc
 	 and free a copy of the entire write request for breakpoint
@@ -1295,15 +1292,10 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
 	 to mitigate this.  */
       len = std::min (ops->to_get_memory_xfer_limit (ops), len);
 
-      buf = (gdb_byte *) xmalloc (len);
-      old_chain = make_cleanup (xfree, buf);
-      memcpy (buf, writebuf, len);
-
-      breakpoint_xfer_memory (NULL, buf, writebuf, memaddr, len);
-      res = memory_xfer_partial_1 (ops, object, NULL, buf, memaddr, len,
+      std::vector<gdb_byte> buf (writebuf, writebuf + len);
+      breakpoint_xfer_memory (NULL, buf.data (), writebuf, memaddr, len);
+      res = memory_xfer_partial_1 (ops, object, NULL, buf.data (), memaddr, len,
 				   xfered_len);
-
-      do_cleanups (old_chain);
     }
 
   return res;
@@ -2439,9 +2431,7 @@ simple_search_memory (struct target_ops *ops,
 #define SEARCH_CHUNK_SIZE 16000
   const unsigned chunk_size = SEARCH_CHUNK_SIZE;
   /* Buffer to hold memory contents for searching.  */
-  gdb_byte *search_buf;
   unsigned search_buf_size;
-  struct cleanup *old_cleanups;
 
   search_buf_size = chunk_size + pattern_len - 1;
 
@@ -2449,20 +2439,17 @@ simple_search_memory (struct target_ops *ops,
   if (search_space_len < search_buf_size)
     search_buf_size = search_space_len;
 
-  search_buf = (gdb_byte *) malloc (search_buf_size);
-  if (search_buf == NULL)
-    error (_("Unable to allocate memory to perform the search."));
-  old_cleanups = make_cleanup (free_current_contents, &search_buf);
+  std::vector<gdb_byte> search_buf (search_buf_size);
 
   /* Prime the search buffer.  */
 
   if (target_read (ops, TARGET_OBJECT_MEMORY, NULL,
-		   search_buf, start_addr, search_buf_size) != search_buf_size)
+		   search_buf.data (), start_addr, search_buf_size)
+      != search_buf_size)
     {
       warning (_("Unable to access %s bytes of target "
 		 "memory at %s, halting search."),
 	       pulongest (search_buf_size), hex_string (start_addr));
-      do_cleanups (old_cleanups);
       return -1;
     }
 
@@ -2478,15 +2465,14 @@ simple_search_memory (struct target_ops *ops,
       unsigned nr_search_bytes
 	= std::min (search_space_len, (ULONGEST) search_buf_size);
 
-      found_ptr = (gdb_byte *) memmem (search_buf, nr_search_bytes,
+      found_ptr = (gdb_byte *) memmem (search_buf.data (), nr_search_bytes,
 				       pattern, pattern_len);
 
       if (found_ptr != NULL)
 	{
-	  CORE_ADDR found_addr = start_addr + (found_ptr - search_buf);
+	  CORE_ADDR found_addr = start_addr + (found_ptr - search_buf.data ());
 
 	  *found_addrp = found_addr;
-	  do_cleanups (old_cleanups);
 	  return 1;
 	}
 
@@ -2507,20 +2493,19 @@ simple_search_memory (struct target_ops *ops,
 	  /* Copy the trailing part of the previous iteration to the front
 	     of the buffer for the next iteration.  */
 	  gdb_assert (keep_len == pattern_len - 1);
-	  memcpy (search_buf, search_buf + chunk_size, keep_len);
+	  memcpy (&search_buf[0], &search_buf[chunk_size], keep_len);
 
 	  nr_to_read = std::min (search_space_len - keep_len,
 				 (ULONGEST) chunk_size);
 
 	  if (target_read (ops, TARGET_OBJECT_MEMORY, NULL,
-			   search_buf + keep_len, read_addr,
+			   &search_buf[keep_len], read_addr,
 			   nr_to_read) != nr_to_read)
 	    {
 	      warning (_("Unable to access %s bytes of target "
 			 "memory at %s, halting search."),
 		       plongest (nr_to_read),
 		       hex_string (read_addr));
-	      do_cleanups (old_cleanups);
 	      return -1;
 	    }
 
@@ -2530,7 +2515,6 @@ simple_search_memory (struct target_ops *ops,
 
   /* Not found.  */
 
-  do_cleanups (old_cleanups);
   return 0;
 }
 
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 985233c..9724aca 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -697,12 +697,9 @@ value_concat (struct value *arg1, struct value *arg2)
       if (TYPE_CODE (type2) == TYPE_CODE_STRING
 	  || TYPE_CODE (type2) == TYPE_CODE_CHAR)
 	{
-	  struct cleanup *back_to;
-
 	  count = longest_to_int (value_as_long (inval1));
 	  inval2len = TYPE_LENGTH (type2);
-	  ptr = (char *) xmalloc (count * inval2len);
-	  back_to = make_cleanup (xfree, ptr);
+	  std::vector<char> ptr (count * inval2len);
 	  if (TYPE_CODE (type2) == TYPE_CODE_CHAR)
 	    {
 	      char_type = type2;
@@ -711,7 +708,7 @@ value_concat (struct value *arg1, struct value *arg2)
 					   value_contents (inval2));
 	      for (idx = 0; idx < count; idx++)
 		{
-		  *(ptr + idx) = inchar;
+		  ptr[idx] = inchar;
 		}
 	    }
 	  else
@@ -720,12 +717,11 @@ value_concat (struct value *arg1, struct value *arg2)
 
 	      for (idx = 0; idx < count; idx++)
 		{
-		  memcpy (ptr + (idx * inval2len), value_contents (inval2),
+		  memcpy (&ptr[idx * inval2len], value_contents (inval2),
 			  inval2len);
 		}
 	    }
-	  outval = value_string (ptr, count * inval2len, char_type);
-	  do_cleanups (back_to);
+	  outval = value_string (ptr.data (), count * inval2len, char_type);
 	}
       else if (TYPE_CODE (type2) == TYPE_CODE_BOOL)
 	{
@@ -739,8 +735,6 @@ value_concat (struct value *arg1, struct value *arg2)
   else if (TYPE_CODE (type1) == TYPE_CODE_STRING
 	   || TYPE_CODE (type1) == TYPE_CODE_CHAR)
     {
-      struct cleanup *back_to;
-
       /* We have two character strings to concatenate.  */
       if (TYPE_CODE (type2) != TYPE_CODE_STRING
 	  && TYPE_CODE (type2) != TYPE_CODE_CHAR)
@@ -749,31 +743,29 @@ value_concat (struct value *arg1, struct value *arg2)
 	}
       inval1len = TYPE_LENGTH (type1);
       inval2len = TYPE_LENGTH (type2);
-      ptr = (char *) xmalloc (inval1len + inval2len);
-      back_to = make_cleanup (xfree, ptr);
+      std::vector<char> ptr (inval1len + inval2len);
       if (TYPE_CODE (type1) == TYPE_CODE_CHAR)
 	{
 	  char_type = type1;
 
-	  *ptr = (char) unpack_long (type1, value_contents (inval1));
+	  ptr[0] = (char) unpack_long (type1, value_contents (inval1));
 	}
       else
 	{
 	  char_type = TYPE_TARGET_TYPE (type1);
 
-	  memcpy (ptr, value_contents (inval1), inval1len);
+	  memcpy (ptr.data (), value_contents (inval1), inval1len);
 	}
       if (TYPE_CODE (type2) == TYPE_CODE_CHAR)
 	{
-	  *(ptr + inval1len) =
+	  ptr[inval1len] =
 	    (char) unpack_long (type2, value_contents (inval2));
 	}
       else
 	{
-	  memcpy (ptr + inval1len, value_contents (inval2), inval2len);
+	  memcpy (&ptr[inval1len], value_contents (inval2), inval2len);
 	}
-      outval = value_string (ptr, inval1len + inval2len, char_type);
-      do_cleanups (back_to);
+      outval = value_string (ptr.data (), inval1len + inval2len, char_type);
     }
   else if (TYPE_CODE (type1) == TYPE_CODE_BOOL)
     {
diff --git a/gdb/valops.c b/gdb/valops.c
index 93ae6cf..69444d8 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2075,24 +2075,20 @@ search_struct_method (const char *name, struct value **arg1p,
 
 	  if (offset < 0 || offset >= TYPE_LENGTH (type))
 	    {
-	      gdb_byte *tmp;
-	      struct cleanup *back_to;
 	      CORE_ADDR address;
 
-	      tmp = (gdb_byte *) xmalloc (TYPE_LENGTH (baseclass));
-	      back_to = make_cleanup (xfree, tmp);
+	      std::vector<gdb_byte> tmp (TYPE_LENGTH (baseclass));
 	      address = value_address (*arg1p);
 
 	      if (target_read_memory (address + offset,
-				      tmp, TYPE_LENGTH (baseclass)) != 0)
+				      tmp.data (), TYPE_LENGTH (baseclass)) != 0)
 		error (_("virtual baseclass botch"));
 
 	      base_val = value_from_contents_and_address (baseclass,
-							  tmp,
+							  tmp.data (),
 							  address + offset);
 	      base_valaddr = value_contents_for_printing (base_val);
 	      this_offset = 0;
-	      do_cleanups (back_to);
 	    }
 	  else
 	    {
-- 
2.9.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]