[RFA] Fix memory leak in add_symbol_file_command

[RFA] Fix memory leak in add_symbol_file_command

[Tom Tromey]

I happened to notice that add_symbol_file_command leaks "sect_opts".
This patch fixes the leak by changing sect_opts to be a std::vector.

I had to change the logic in the loop a little bit.  Previously, it
was incrementing section_index after completing an entry; but this
changes it to push a new entry when the name is seen.

I believe the argument parsing here is mildly incorrect, in that
nothing checks whether the -s option actually had any arguments.
Maybe gdb can crash if "-s NAME" is given without an argument.  I
didn't try to fix this in this patch, but I do have another patch I
can send later that fixes it up.

Regression tested on the buildbot.

ChangeLog
2017-08-11  Tom Tromey  <tom@tromey.com>

	* symfile.c (add_symbol_file_command): Use std::vector.
---
 gdb/ChangeLog |  4 ++++
 gdb/symfile.c | 43 +++++++++++--------------------------------
 2 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c588291..20f6d60 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2017-08-11  Tom Tromey  <tom@tromey.com>
+
+	* symfile.c (add_symbol_file_command): Use std::vector.
+
 2017-08-11  Pedro Alves  <palves@redhat.com>
 
 	* infrun.c (process_event_stop_test): Adjust
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 3e2df9b..4158dd4 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2205,10 +2205,8 @@ add_symbol_file_command (char *args, int from_tty)
   struct gdbarch *gdbarch = get_current_arch ();
   gdb::unique_xmalloc_ptr<char> filename;
   char *arg;
-  int section_index = 0;
   int argcnt = 0;
   int sec_num = 0;
-  int i;
   int expecting_sec_name = 0;
   int expecting_sec_addr = 0;
   struct objfile *objf;
@@ -2225,13 +2223,9 @@ add_symbol_file_command (char *args, int from_tty)
   };
 
   struct section_addr_info *section_addrs;
-  struct sect_opt *sect_opts = NULL;
-  size_t num_sect_opts = 0;
+  std::vector<struct sect_opt> sect_opts;
   struct cleanup *my_cleanups = make_cleanup (null_cleanup, NULL);
 
-  num_sect_opts = 16;
-  sect_opts = XNEWVEC (struct sect_opt, num_sect_opts);
-
   dont_repeat ();
 
   if (args == NULL)
@@ -2251,16 +2245,8 @@ add_symbol_file_command (char *args, int from_tty)
 	{
 	  /* The second argument is always the text address at which
 	     to load the program.  */
-	  sect_opts[section_index].name = ".text";
-	  sect_opts[section_index].value = arg;
-	  if (++section_index >= num_sect_opts)
-	    {
-	      num_sect_opts *= 2;
-	      sect_opts = ((struct sect_opt *)
-			   xrealloc (sect_opts,
-				     num_sect_opts
-				     * sizeof (struct sect_opt)));
-	    }
+	  struct sect_opt sect = { ".text", arg };
+	  sect_opts.push_back (sect);
 	}
       else
 	{
@@ -2268,21 +2254,14 @@ add_symbol_file_command (char *args, int from_tty)
 	     to an option.  */
 	  if (expecting_sec_name)
 	    {
-	      sect_opts[section_index].name = arg;
+	      struct sect_opt sect = { arg, NULL };
+	      sect_opts.push_back (sect);
 	      expecting_sec_name = 0;
 	    }
 	  else if (expecting_sec_addr)
 	    {
-	      sect_opts[section_index].value = arg;
+	      sect_opts.back ().value = arg;
 	      expecting_sec_addr = 0;
-	      if (++section_index >= num_sect_opts)
-		{
-		  num_sect_opts *= 2;
-		  sect_opts = ((struct sect_opt *)
-			       xrealloc (sect_opts,
-					 num_sect_opts
-					 * sizeof (struct sect_opt)));
-		}
 	    }
 	  else if (strcmp (arg, "-readnow") == 0)
 	    flags |= OBJF_READNOW;
@@ -2301,7 +2280,7 @@ add_symbol_file_command (char *args, int from_tty)
      filename, and the second is the address where this file has been
      loaded.  Abort now if this address hasn't been provided by the
      user.  */
-  if (section_index < 1)
+  if (sect_opts.size () < 1)
     error (_("The address where %s has been loaded is missing"),
 	   filename.get ());
 
@@ -2313,13 +2292,13 @@ add_symbol_file_command (char *args, int from_tty)
 
   printf_unfiltered (_("add symbol table from file \"%s\" at\n"),
 		     filename.get ());
-  section_addrs = alloc_section_addr_info (section_index);
+  section_addrs = alloc_section_addr_info (sect_opts.size ());
   make_cleanup (xfree, section_addrs);
-  for (i = 0; i < section_index; i++)
+  for (sect_opt &sect : sect_opts)
     {
       CORE_ADDR addr;
-      const char *val = sect_opts[i].value;
-      const char *sec = sect_opts[i].name;
+      const char *val = sect.value;
+      const char *sec = sect.name;
 
       addr = parse_and_eval_address (val);
 
-- 
2.9.4

Re: [RFA] Fix memory leak in add_symbol_file_command

[Pedro Alves]

Looks fine to me.  A couple nits.

On 08/11/2017 10:18 PM, Tom Tromey wrote:

>    struct section_addr_info *section_addrs;
> -  struct sect_opt *sect_opts = NULL;
> -  size_t num_sect_opts = 0;
> +  std::vector<struct sect_opt> sect_opts;

Drop "struct" throughout in the new code?  You're already
dropping it here:

 > -  for (i = 0; i < section_index; i++)
 > +  for (sect_opt &sect : sect_opts)

> @@ -2301,7 +2280,7 @@ add_symbol_file_command (char *args, int from_tty)
>       filename, and the second is the address where this file has been
>       loaded.  Abort now if this address hasn't been provided by the
>       user.  */
> -  if (section_index < 1)
> +  if (sect_opts.size () < 1)
>      error (_("The address where %s has been loaded is missing"),
>  	   filename.get ());

  'if (sect_opts.empty ())'  ?

> -  for (i = 0; i < section_index; i++)
> +  for (sect_opt &sect : sect_opts)
>      {
>        CORE_ADDR addr;
> -      const char *val = sect_opts[i].value;
> -      const char *sec = sect_opts[i].name;
> +      const char *val = sect.value;
> +      const char *sec = sect.name;

Thanks,
Pedro Alves

Re: [RFA] Fix memory leak in add_symbol_file_command

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Looks fine to me.  A couple nits.

Thanks.  I fixed these, and I'm checking it in.

Tom