[PATCH 0/3] C++ification of symfile_segment_data

[PATCH 0/3] C++ification of symfile_segment_data

[Simon Marchi]

As I was reading this code, I did this simple translation from manual
memory management to using C++ constructs.

Simon Marchi (3):
  gdb: allocate symfile_segment_data with new
  gdb: use std::vector to store segments in symfile_segment_data
  gdb: make symfile_segment_data::segment_info an std::vector

 gdb/elfread.c       | 17 ++++++-----------
 gdb/remote.c        | 22 ++++++++++------------
 gdb/solib-target.c  | 18 ++++++++----------
 gdb/symfile-debug.c |  2 +-
 gdb/symfile.c       | 45 ++++++++++++++-------------------------------
 gdb/symfile.h       | 41 +++++++++++++++++++++++------------------
 6 files changed, 62 insertions(+), 83 deletions(-)

-- 
2.26.2

[PATCH 1/3] gdb: allocate symfile_segment_data with new

[Simon Marchi]

- Allocate this structure with new instead of XNEW, use a unique pointer
  to manage its lifetime.
- Change a few functions to return a unique pointer instead of a
  plain pointer.
- Change free_symfile_segment_data to be symfile_segment_data's
  destructor.

gdb/ChangeLog:

	* symfile.h (struct symfile_segment_data): Initialize fields.
	<~symfile_segment_data>: Add.
	(symfile_segment_data_up): New.
	(struct sym_fns) <sym_segments>: Return a
	symfile_segment_data_up.
	(default_symfile_segments): Return a symfile_segment_data_up.
	(free_symfile_segment_data): Remove.
	(get_symfile_segment_data): Return a symfile_segment_data_up.
	* symfile.c (default_symfile_segments): Likewise.
	(get_symfile_segment_data): Likewise.
	(free_symfile_segment_data): Remove.
	(symfile_find_segment_sections): Update.
	* elfread.c (elf_symfile_segments): Return a
	symfile_segment_data_up.
	* remote.c (remote_target::get_offsets): Update.
	* solib-target.c (solib_target_relocate_section_addresses):
	Update.
	* symfile-debug.c (debug_sym_segments): Return a
	symfile_segment_data_up.
---
 gdb/elfread.c       |  5 ++---
 gdb/remote.c        | 12 +++++-------
 gdb/solib-target.c  |  8 +++-----
 gdb/symfile-debug.c |  2 +-
 gdb/symfile.c       | 27 ++++++---------------------
 gdb/symfile.h       | 24 ++++++++++++++++--------
 6 files changed, 33 insertions(+), 45 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 2f2fef93996a..4804d62074c4 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -85,14 +85,13 @@ static const struct bfd_key<elfread_data> probe_key;
 
 /* Locate the segments in ABFD.  */
 
-static struct symfile_segment_data *
+static symfile_segment_data_up
 elf_symfile_segments (bfd *abfd)
 {
   Elf_Internal_Phdr *phdrs, **segments;
   long phdrs_size;
   int num_phdrs, num_segments, num_sections, i;
   asection *sect;
-  struct symfile_segment_data *data;
 
   phdrs_size = bfd_get_elf_phdr_upper_bound (abfd);
   if (phdrs_size == -1)
@@ -112,7 +111,7 @@ elf_symfile_segments (bfd *abfd)
   if (num_segments == 0)
     return NULL;
 
-  data = XCNEW (struct symfile_segment_data);
+  symfile_segment_data_up data (new symfile_segment_data);
   data->num_segments = num_segments;
   data->segment_bases = XCNEWVEC (CORE_ADDR, num_segments);
   data->segment_sizes = XCNEWVEC (CORE_ADDR, num_segments);
diff --git a/gdb/remote.c b/gdb/remote.c
index 812ab8bc1b3b..a28f34d157a9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4105,7 +4105,6 @@ remote_target::get_offsets ()
   char *ptr;
   int lose, num_segments = 0, do_sections, do_segments;
   CORE_ADDR text_addr, data_addr, bss_addr, segments[2];
-  struct symfile_segment_data *data;
 
   if (symfile_objfile == NULL)
     return;
@@ -4185,7 +4184,8 @@ remote_target::get_offsets ()
 
   section_offsets offs = symfile_objfile->section_offsets;
 
-  data = get_symfile_segment_data (symfile_objfile->obfd);
+  symfile_segment_data_up data
+    = get_symfile_segment_data (symfile_objfile->obfd);
   do_segments = (data != NULL);
   do_sections = num_segments == 0;
 
@@ -4220,8 +4220,9 @@ remote_target::get_offsets ()
 
   if (do_segments)
     {
-      int ret = symfile_map_offsets_to_segments (symfile_objfile->obfd, data,
-						 offs, num_segments, segments);
+      int ret = symfile_map_offsets_to_segments (symfile_objfile->obfd,
+						 data.get (), offs,
+						 num_segments, segments);
 
       if (ret == 0 && !do_sections)
 	error (_("Can not handle qOffsets TextSeg "
@@ -4231,9 +4232,6 @@ remote_target::get_offsets ()
 	do_sections = 0;
     }
 
-  if (data)
-    free_symfile_segment_data (data);
-
   if (do_sections)
     {
       offs[SECT_OFF_TEXT (symfile_objfile)] = text_addr;
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index 93d95fdda652..35e50a3e00b4 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -364,9 +364,9 @@ Could not relocate shared library \"%s\": wrong number of ALLOC sections"),
 	}
       else if (!li->segment_bases.empty ())
 	{
-	  struct symfile_segment_data *data;
+	  symfile_segment_data_up data
+	    = get_symfile_segment_data (so->abfd);
 
-	  data = get_symfile_segment_data (so->abfd);
 	  if (data == NULL)
 	    warning (_("\
 Could not relocate shared library \"%s\": no segments"), so->so_name);
@@ -375,7 +375,7 @@ Could not relocate shared library \"%s\": no segments"), so->so_name);
 	      ULONGEST orig_delta;
 	      int i;
 
-	      if (!symfile_map_offsets_to_segments (so->abfd, data,
+	      if (!symfile_map_offsets_to_segments (so->abfd, data.get (),
 						    li->offsets,
 						    li->segment_bases.size (),
 						    li->segment_bases.data ()))
@@ -407,8 +407,6 @@ Could not relocate shared library \"%s\": bad offsets"), so->so_name);
 			       + data->segment_sizes[i - 1]
 			       + orig_delta);
 	      gdb_assert (so->addr_low <= so->addr_high);
-
-	      free_symfile_segment_data (data);
 	    }
 	}
     }
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 203609c326bc..bd806fdfee45 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -475,7 +475,7 @@ debug_sym_offsets (struct objfile *objfile,
   debug_data->real_sf->sym_offsets (objfile, info);
 }
 
-static struct symfile_segment_data *
+static symfile_segment_data_up
 debug_sym_segments (bfd *abfd)
 {
   /* This API function is annoying, it doesn't take a "this" pointer.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 946443f07a89..9d5e2824b2a7 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -717,12 +717,11 @@ default_symfile_offsets (struct objfile *objfile,
    It assumes that object files do not have segments, and fully linked
    files have a single segment.  */
 
-struct symfile_segment_data *
+symfile_segment_data_up
 default_symfile_segments (bfd *abfd)
 {
   int num_sections, i;
   asection *sect;
-  struct symfile_segment_data *data;
   CORE_ADDR low, high;
 
   /* Relocatable files contain enough information to position each
@@ -745,7 +744,7 @@ default_symfile_segments (bfd *abfd)
   low = bfd_section_vma (sect);
   high = low + bfd_section_size (sect);
 
-  data = XCNEW (struct symfile_segment_data);
+  symfile_segment_data_up data (new symfile_segment_data);
   data->num_segments = 1;
   data->segment_bases = XCNEW (CORE_ADDR);
   data->segment_sizes = XCNEW (CORE_ADDR);
@@ -3621,7 +3620,7 @@ symfile_relocate_debug_section (struct objfile *objfile,
   return (*objfile->sf->sym_relocate) (objfile, sectp, buf);
 }
 
-struct symfile_segment_data *
+symfile_segment_data_up
 get_symfile_segment_data (bfd *abfd)
 {
   const struct sym_fns *sf = find_sym_fns (abfd);
@@ -3632,15 +3631,6 @@ get_symfile_segment_data (bfd *abfd)
   return sf->sym_segments (abfd);
 }
 
-void
-free_symfile_segment_data (struct symfile_segment_data *data)
-{
-  xfree (data->segment_bases);
-  xfree (data->segment_sizes);
-  xfree (data->segment_info);
-  xfree (data);
-}
-
 /* Given:
    - DATA, containing segment addresses from the object file ABFD, and
      the mapping from ABFD's sections onto the segments that own them,
@@ -3703,17 +3693,14 @@ symfile_find_segment_sections (struct objfile *objfile)
   bfd *abfd = objfile->obfd;
   int i;
   asection *sect;
-  struct symfile_segment_data *data;
 
-  data = get_symfile_segment_data (objfile->obfd);
+  symfile_segment_data_up data
+    = get_symfile_segment_data (objfile->obfd);
   if (data == NULL)
     return;
 
   if (data->num_segments != 1 && data->num_segments != 2)
-    {
-      free_symfile_segment_data (data);
-      return;
-    }
+    return;
 
   for (i = 0, sect = abfd->sections; sect != NULL; i++, sect = sect->next)
     {
@@ -3736,8 +3723,6 @@ symfile_find_segment_sections (struct objfile *objfile)
 	    objfile->sect_index_bss = sect->index;
 	}
     }
-
-  free_symfile_segment_data (data);
 }
 
 /* Listen for free_objfile events.  */
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 5ada6c370e7e..2dfa6556d479 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -80,26 +80,35 @@ typedef std::vector<other_sections> section_addr_info;
    each BFD section belongs to.  */
 struct symfile_segment_data
 {
+  ~symfile_segment_data ()
+  {
+    xfree (this->segment_bases);
+    xfree (this->segment_sizes);
+    xfree (this->segment_info);
+  }
+
   /* How many segments are present in this file.  If there are
      two, the text segment is the first one and the data segment
      is the second one.  */
-  int num_segments;
+  int num_segments = 0;
 
   /* If NUM_SEGMENTS is greater than zero, the original base address
      of each segment.  */
-  CORE_ADDR *segment_bases;
+  CORE_ADDR *segment_bases = nullptr;
 
   /* If NUM_SEGMENTS is greater than zero, the memory size of each
      segment.  */
-  CORE_ADDR *segment_sizes;
+  CORE_ADDR *segment_sizes = nullptr;
 
   /* If NUM_SEGMENTS is greater than zero, this is an array of entries
      recording which segment contains each BFD section.
      SEGMENT_INFO[I] is S+1 if the I'th BFD section belongs to segment
      S, or zero if it is not in any segment.  */
-  int *segment_info;
+  int *segment_info = nullptr;
 };
 
+using symfile_segment_data_up = std::unique_ptr<symfile_segment_data>;
+
 /* Callback for quick_symbol_functions->map_symbol_filenames.  */
 
 typedef void (symbol_filename_ftype) (const char *filename,
@@ -360,7 +369,7 @@ struct sym_fns
      the segments of ABFD.  Each segment is a unit of the file
      which may be relocated independently.  */
 
-  struct symfile_segment_data *(*sym_segments) (bfd *abfd);
+  symfile_segment_data_up (*sym_segments) (bfd *abfd);
 
   /* This function should read the linetable from the objfile when
      the line table cannot be read while processing the debugging
@@ -401,7 +410,7 @@ extern void default_symfile_offsets (struct objfile *objfile,
 /* The default version of sym_fns.sym_segments for readers that don't
    do anything special.  */
 
-extern struct symfile_segment_data *default_symfile_segments (bfd *abfd);
+extern symfile_segment_data_up default_symfile_segments (bfd *abfd);
 
 /* The default version of sym_fns.sym_relocate for readers that don't
    do anything special.  */
@@ -530,8 +539,7 @@ extern int symfile_map_offsets_to_segments (bfd *,
 					    const struct symfile_segment_data *,
 					    section_offsets &,
 					    int, const CORE_ADDR *);
-struct symfile_segment_data *get_symfile_segment_data (bfd *abfd);
-void free_symfile_segment_data (struct symfile_segment_data *data);
+symfile_segment_data_up get_symfile_segment_data (bfd *abfd);
 
 extern scoped_restore_tmpl<int> increment_reading_symtab (void);
 
-- 
2.26.2

[PATCH 2/3] gdb: use std::vector to store segments in symfile_segment_data

[Simon Marchi]

Instead of maintaining two vectors, I added a small `segment` class
which holds both the base address and size of one segment and replaced
the two `segment_bases` and `segment_sizes` arrays with a single vector.

The rest of the changes are straightforward, no behavior changes are
expected.

gdb/ChangeLog:

	* symfile.h (struct symfile_segment_data) <struct segment>: New.
	<segments>: New.
	<segment_bases, segment_sizes>: Remove.
	* symfile.c (default_symfile_segments): Update.
	* elfread.c (elf_symfile_segments): Update.
	* remote.c (remote_target::get_offsets): Update.
	* solib-target.c (solib_target_relocate_section_addresses):
	Update.
---
 gdb/elfread.c      |  8 +-------
 gdb/remote.c       | 10 +++++-----
 gdb/solib-target.c | 10 +++++-----
 gdb/symfile.c      | 14 +++++---------
 gdb/symfile.h      | 32 +++++++++++++++++---------------
 5 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 4804d62074c4..822f443fa8d3 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -112,15 +112,9 @@ elf_symfile_segments (bfd *abfd)
     return NULL;
 
   symfile_segment_data_up data (new symfile_segment_data);
-  data->num_segments = num_segments;
-  data->segment_bases = XCNEWVEC (CORE_ADDR, num_segments);
-  data->segment_sizes = XCNEWVEC (CORE_ADDR, num_segments);
 
   for (i = 0; i < num_segments; i++)
-    {
-      data->segment_bases[i] = segments[i]->p_vaddr;
-      data->segment_sizes[i] = segments[i]->p_memsz;
-    }
+    data->segments.emplace_back (segments[i]->p_vaddr, segments[i]->p_memsz);
 
   num_sections = bfd_count_sections (abfd);
   data->segment_info = XCNEWVEC (int, num_sections);
diff --git a/gdb/remote.c b/gdb/remote.c
index a28f34d157a9..312a03c8fb42 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4198,10 +4198,10 @@ remote_target::get_offsets ()
      by assuming that the .text and .data offsets apply to the whole
      text and data segments.  Convert the offsets given in the packet
      to base addresses for symfile_map_offsets_to_segments.  */
-  else if (data && data->num_segments == 2)
+  else if (data != nullptr && data->segments.size () == 2)
     {
-      segments[0] = data->segment_bases[0] + text_addr;
-      segments[1] = data->segment_bases[1] + data_addr;
+      segments[0] = data->segments[0].base + text_addr;
+      segments[1] = data->segments[1].base + data_addr;
       num_segments = 2;
     }
   /* If the object file has only one segment, assume that it is text
@@ -4209,9 +4209,9 @@ remote_target::get_offsets ()
      but programs with no code are useless.  Of course the code might
      have ended up in the data segment... to detect that we would need
      the permissions here.  */
-  else if (data && data->num_segments == 1)
+  else if (data && data->segments.size () == 1)
     {
-      segments[0] = data->segment_bases[0] + text_addr;
+      segments[0] = data->segments[0].base + text_addr;
       num_segments = 1;
     }
   /* There's no way to relocate by segment.  */
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index 35e50a3e00b4..ba056478c061 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -386,9 +386,9 @@ Could not relocate shared library \"%s\": bad offsets"), so->so_name);
 		 "info sharedlibrary".  Report any consecutive segments
 		 which were relocated as a single unit.  */
 	      gdb_assert (li->segment_bases.size () > 0);
-	      orig_delta = li->segment_bases[0] - data->segment_bases[0];
+	      orig_delta = li->segment_bases[0] - data->segments[0].base;
 
-	      for (i = 1; i < data->num_segments; i++)
+	      for (i = 1; i < data->segments.size (); i++)
 		{
 		  /* If we have run out of offsets, assume all
 		     remaining segments have the same offset.  */
@@ -397,14 +397,14 @@ Could not relocate shared library \"%s\": bad offsets"), so->so_name);
 
 		  /* If this segment does not have the same offset, do
 		     not include it in the library's range.  */
-		  if (li->segment_bases[i] - data->segment_bases[i]
+		  if (li->segment_bases[i] - data->segments[i].base
 		      != orig_delta)
 		    break;
 		}
 
 	      so->addr_low = li->segment_bases[0];
-	      so->addr_high = (data->segment_bases[i - 1]
-			       + data->segment_sizes[i - 1]
+	      so->addr_high = (data->segments[i - 1].base
+			       + data->segments[i - 1].size
 			       + orig_delta);
 	      gdb_assert (so->addr_low <= so->addr_high);
 	    }
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 9d5e2824b2a7..22793e736398 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -745,9 +745,6 @@ default_symfile_segments (bfd *abfd)
   high = low + bfd_section_size (sect);
 
   symfile_segment_data_up data (new symfile_segment_data);
-  data->num_segments = 1;
-  data->segment_bases = XCNEW (CORE_ADDR);
-  data->segment_sizes = XCNEW (CORE_ADDR);
 
   num_sections = bfd_count_sections (abfd);
   data->segment_info = XCNEWVEC (int, num_sections);
@@ -768,8 +765,7 @@ default_symfile_segments (bfd *abfd)
       data->segment_info[i] = 1;
     }
 
-  data->segment_bases[0] = low;
-  data->segment_sizes[0] = high - low;
+  data->segments.emplace_back (low, high - low);
 
   return data;
 }
@@ -3663,13 +3659,13 @@ symfile_map_offsets_to_segments (bfd *abfd,
   /* If we do not have segment mappings for the object file, we
      can not relocate it by segments.  */
   gdb_assert (data != NULL);
-  gdb_assert (data->num_segments > 0);
+  gdb_assert (data->segments.size () > 0);
 
   for (i = 0, sect = abfd->sections; sect != NULL; i++, sect = sect->next)
     {
       int which = data->segment_info[i];
 
-      gdb_assert (0 <= which && which <= data->num_segments);
+      gdb_assert (0 <= which && which <= data->segments.size ());
 
       /* Don't bother computing offsets for sections that aren't
          loaded as part of any segment.  */
@@ -3681,7 +3677,7 @@ symfile_map_offsets_to_segments (bfd *abfd,
       if (which > num_segment_bases)
         which = num_segment_bases;
 
-      offsets[i] = segment_bases[which - 1] - data->segment_bases[which - 1];
+      offsets[i] = segment_bases[which - 1] - data->segments[which - 1].base;
     }
 
   return 1;
@@ -3699,7 +3695,7 @@ symfile_find_segment_sections (struct objfile *objfile)
   if (data == NULL)
     return;
 
-  if (data->num_segments != 1 && data->num_segments != 2)
+  if (data->segments.size () != 1 && data->segments.size () != 2)
     return;
 
   for (i = 0, sect = abfd->sections; sect != NULL; i++, sect = sect->next)
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 2dfa6556d479..1f2395169af9 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -80,29 +80,31 @@ typedef std::vector<other_sections> section_addr_info;
    each BFD section belongs to.  */
 struct symfile_segment_data
 {
+  struct segment
+  {
+    segment (CORE_ADDR base, CORE_ADDR size)
+      : base (base), size (size)
+    {}
+
+    /* The original base address the segment.  */
+    CORE_ADDR base;
+
+    /* The memory size of the segment.  */
+    CORE_ADDR size;
+  };
+
   ~symfile_segment_data ()
   {
-    xfree (this->segment_bases);
-    xfree (this->segment_sizes);
     xfree (this->segment_info);
   }
 
-  /* How many segments are present in this file.  If there are
+  /* The segments present in this file.  If there are
      two, the text segment is the first one and the data segment
      is the second one.  */
-  int num_segments = 0;
-
-  /* If NUM_SEGMENTS is greater than zero, the original base address
-     of each segment.  */
-  CORE_ADDR *segment_bases = nullptr;
-
-  /* If NUM_SEGMENTS is greater than zero, the memory size of each
-     segment.  */
-  CORE_ADDR *segment_sizes = nullptr;
+  std::vector<segment> segments;
 
-  /* If NUM_SEGMENTS is greater than zero, this is an array of entries
-     recording which segment contains each BFD section.
-     SEGMENT_INFO[I] is S+1 if the I'th BFD section belongs to segment
+  /* This is an array of entries recording which segment contains each BFD
+     section.  SEGMENT_INFO[I] is S+1 if the I'th BFD section belongs to segment
      S, or zero if it is not in any segment.  */
   int *segment_info = nullptr;
 };
-- 
2.26.2

[PATCH 3/3] gdb: make symfile_segment_data::segment_info an std::vector

[Simon Marchi]

Change the symfile_segment_data::segment_info array to be an
std::vector.  No functional changes are expected.

gdb/ChangeLog:

	* symfile.h (struct symfile_segment_data)
	<~symfile_segment_data>: Remove.
	<segment_info>: Change to std::vector.
	* symfile.c (default_symfile_segments): Update.
	* elfread.c (elf_symfile_segments): Update.
---
 gdb/elfread.c | 4 +++-
 gdb/symfile.c | 4 +++-
 gdb/symfile.h | 7 +------
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index 822f443fa8d3..532fe96dcdca 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -117,7 +117,9 @@ elf_symfile_segments (bfd *abfd)
     data->segments.emplace_back (segments[i]->p_vaddr, segments[i]->p_memsz);
 
   num_sections = bfd_count_sections (abfd);
-  data->segment_info = XCNEWVEC (int, num_sections);
+
+  /* All elements are initialized to 0 (map to no segment).  */
+  data->segment_info.resize (num_sections);
 
   for (i = 0, sect = abfd->sections; sect != NULL; i++, sect = sect->next)
     {
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 22793e736398..e6ec458504af 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -747,7 +747,9 @@ default_symfile_segments (bfd *abfd)
   symfile_segment_data_up data (new symfile_segment_data);
 
   num_sections = bfd_count_sections (abfd);
-  data->segment_info = XCNEWVEC (int, num_sections);
+
+  /* All elements are initialized to 0 (map to no segment).  */
+  data->segment_info.resize (num_sections);
 
   for (i = 0, sect = abfd->sections; sect != NULL; i++, sect = sect->next)
     {
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 1f2395169af9..fe79f79a0482 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -93,11 +93,6 @@ struct symfile_segment_data
     CORE_ADDR size;
   };
 
-  ~symfile_segment_data ()
-  {
-    xfree (this->segment_info);
-  }
-
   /* The segments present in this file.  If there are
      two, the text segment is the first one and the data segment
      is the second one.  */
@@ -106,7 +101,7 @@ struct symfile_segment_data
   /* This is an array of entries recording which segment contains each BFD
      section.  SEGMENT_INFO[I] is S+1 if the I'th BFD section belongs to segment
      S, or zero if it is not in any segment.  */
-  int *segment_info = nullptr;
+  std::vector<int> segment_info;
 };
 
 using symfile_segment_data_up = std::unique_ptr<symfile_segment_data>;
-- 
2.26.2

Re: [PATCH 2/3] gdb: use std::vector to store segments in symfile_segment_data

[Simon Marchi]

On 2020-05-19 10:55 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon>    symfile_segment_data_up data (new symfile_segment_data);
> Simon> -  data->num_segments = num_segments;
> Simon> -  data->segment_bases = XCNEWVEC (CORE_ADDR, num_segments);
> Simon> -  data->segment_sizes = XCNEWVEC (CORE_ADDR, num_segments);
> 
> Simon>    for (i = 0; i < num_segments; i++)
> Simon> -    {
> Simon> -      data->segment_bases[i] = segments[i]->p_vaddr;
> Simon> -      data->segment_sizes[i] = segments[i]->p_memsz;
> Simon> -    }
> Simon> +    data->segments.emplace_back (segments[i]->p_vaddr, segments[i]->p_memsz);
> 
> It's probably better to just resize() to the desired size, instead of
> using emplace_back.  That may save some memory, because it won't
> over-allocate.
> 
> Probably not super important, but at the same time easy to do.

Good point, will do.  I probably can't resize if the elements are not
default constructible, but if I call reserve, it should work as well?

Simon

Re: [PATCH 2/3] gdb: use std::vector to store segments in symfile_segment_data

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon>    symfile_segment_data_up data (new symfile_segment_data);
Simon> -  data->num_segments = num_segments;
Simon> -  data->segment_bases = XCNEWVEC (CORE_ADDR, num_segments);
Simon> -  data->segment_sizes = XCNEWVEC (CORE_ADDR, num_segments);

Simon>    for (i = 0; i < num_segments; i++)
Simon> -    {
Simon> -      data->segment_bases[i] = segments[i]->p_vaddr;
Simon> -      data->segment_sizes[i] = segments[i]->p_memsz;
Simon> -    }
Simon> +    data->segments.emplace_back (segments[i]->p_vaddr, segments[i]->p_memsz);

It's probably better to just resize() to the desired size, instead of
using emplace_back.  That may save some memory, because it won't
over-allocate.

Probably not super important, but at the same time easy to do.

Tom

Re: [PATCH 0/3] C++ification of symfile_segment_data

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> As I was reading this code, I did this simple translation from manual
Simon> memory management to using C++ constructs.

I sent one nit, but this all looks good to me.

Tom

Re: [PATCH 2/3] gdb: use std::vector to store segments in symfile_segment_data

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> Good point, will do.  I probably can't resize if the elements are not
Simon> default constructible, but if I call reserve, it should work as well?

Yeah, that seems reasonable.

Tom

Re: [PATCH 0/3] C++ification of symfile_segment_data

[Simon Marchi]

On 2020-05-19 10:57 a.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> As I was reading this code, I did this simple translation from manual
> Simon> memory management to using C++ constructs.
> 
> I sent one nit, but this all looks good to me.
> 
> Tom
> 

I pushed it after adding the "reserve" in the second patch.  Thanks!

Simon