[RFA] Remove a cleanup from call_function_by_hand_dummy

Message ID 20180217161104.4909-1-tom@tromey.com
State New
Headers show
Series
  • [RFA] Remove a cleanup from call_function_by_hand_dummy
Related show

Commit Message

Tom Tromey Feb. 17, 2018, 4:11 p.m.
This removes a cleanup from call_function_by_hand_dummy, replacing
manual allocation with std::vector.

Regression tested by the buildbot.

gdb/ChangeLog
2018-02-16  Tom Tromey  <tom@tromey.com>

	* infcall.c (call_function_by_hand_dummy): Use std::vector.
---
 gdb/ChangeLog |  4 ++++
 gdb/infcall.c | 14 ++++----------
 2 files changed, 8 insertions(+), 10 deletions(-)

-- 
2.13.6

Comments

Simon Marchi Feb. 17, 2018, 7:23 p.m. | #1
On 2018-02-17 11:11, Tom Tromey wrote:
> This removes a cleanup from call_function_by_hand_dummy, replacing

> manual allocation with std::vector.

> 

> Regression tested by the buildbot.

> 

> gdb/ChangeLog

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

> 

> 	* infcall.c (call_function_by_hand_dummy): Use std::vector.

> ---

>  gdb/ChangeLog |  4 ++++

>  gdb/infcall.c | 14 ++++----------

>  2 files changed, 8 insertions(+), 10 deletions(-)

> 

> diff --git a/gdb/infcall.c b/gdb/infcall.c

> index 8b75297251..6ff725a815 100644

> --- a/gdb/infcall.c

> +++ b/gdb/infcall.c

> @@ -732,7 +732,6 @@ call_function_by_hand_dummy (struct value 

> *function,

>    struct type *ftype = check_typedef (value_type (function));

>    CORE_ADDR bp_addr;

>    struct frame_id dummy_id;

> -  struct cleanup *args_cleanup;

>    struct frame_info *frame;

>    struct gdbarch *gdbarch;

>    struct cleanup *terminate_bp_cleanup;

> @@ -1054,21 +1053,18 @@ call_function_by_hand_dummy (struct value 

> *function,

>  	}

>      }

> 

> +  std::vector<struct value *> new_args;

>    if (hidden_first_param_p)

>      {

> -      struct value **new_args;

> -

>        /* Add the new argument to the front of the argument list.  */

> -      new_args = XNEWVEC (struct value *, nargs + 1);

> +      new_args.reserve (nargs + 1);


This should probably be resize instead of reserve.  When you call 
reserve, it doesn't change the actual length of the vector.  You could 
then use a def_vector to avoid unneeded zero-initialization by the 
resize operation.

Other than that, LGTM.

Simon
Tom Tromey Feb. 17, 2018, 8:47 p.m. | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:


Simon> This should probably be resize instead of reserve.  When you call
Simon> reserve, it doesn't change the actual length of the vector.  You could
Simon> then use a def_vector to avoid unneeded zero-initialization by the
Simon> resize operation.

I think I get these mixed up 100% of the time :(
What's worse is I read the docs and I guess misunderstood them again.

Maybe I should avoid this entirely by using std::copy or something like
that.

Tom
Tom Tromey Feb. 21, 2018, 3:54 a.m. | #3
Tom> Maybe I should avoid this entirely by using std::copy or something like
Tom> that.

How about this instead?

Tom

commit 7377e0d67ff5f26e9faed818a76e9b7b2727a963
Author: Tom Tromey <tom@tromey.com>
Date:   Fri Feb 16 16:11:29 2018 -0700

    Remove a cleanup from call_function_by_hand_dummy
    
    This removes a cleanup from call_function_by_hand_dummy, replacing
    manual allocation with std::vector.
    
    Regression tested by the buildbot.
    
    gdb/ChangeLog
    2018-02-16  Tom Tromey  <tom@tromey.com>
    
            * infcall.c (call_function_by_hand_dummy): Use std::vector.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9f78ed33cc..8b91d73b0b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2018-02-16  Tom Tromey  <tom@tromey.com>
+
+	* infcall.c (call_function_by_hand_dummy): Use std::vector.
+
 2018-02-20  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* remote-sim.c (gdb_os_printf_filtered, gdb_os_vprintf_filtered,
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 8b75297251..b7f4a176db 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -39,6 +39,7 @@
 #include "top.h"
 #include "interps.h"
 #include "thread-fsm.h"
+#include <algorithm>
 
 /* If we can't find a function's name from its address,
    we print this instead.  */
@@ -732,7 +733,6 @@ call_function_by_hand_dummy (struct value *function,
   struct type *ftype = check_typedef (value_type (function));
   CORE_ADDR bp_addr;
   struct frame_id dummy_id;
-  struct cleanup *args_cleanup;
   struct frame_info *frame;
   struct gdbarch *gdbarch;
   struct cleanup *terminate_bp_cleanup;
@@ -1054,21 +1054,16 @@ call_function_by_hand_dummy (struct value *function,
 	}
     }
 
+  std::vector<struct value *> new_args;
   if (hidden_first_param_p)
     {
-      struct value **new_args;
-
       /* Add the new argument to the front of the argument list.  */
-      new_args = XNEWVEC (struct value *, nargs + 1);
-      new_args[0] = value_from_pointer (lookup_pointer_type (values_type),
-					struct_addr);
-      memcpy (&new_args[1], &args[0], sizeof (struct value *) * nargs);
-      args = new_args;
+      new_args.push_back
+	(value_from_pointer (lookup_pointer_type (values_type), struct_addr));
+      std::copy (&args[0], &args[nargs], std::back_inserter (new_args));
+      args = new_args.data ();
       nargs++;
-      args_cleanup = make_cleanup (xfree, args);
     }
-  else
-    args_cleanup = make_cleanup (null_cleanup, NULL);
 
   /* Create the dummy stack frame.  Pass in the call dummy address as,
      presumably, the ABI code knows where, in the call dummy, the
@@ -1077,8 +1072,6 @@ call_function_by_hand_dummy (struct value *function,
 				bp_addr, nargs, args,
 				sp, struct_return, struct_addr);
 
-  do_cleanups (args_cleanup);
-
   /* Set up a frame ID for the dummy frame so we can pass it to
      set_momentary_breakpoint.  We need to give the breakpoint a frame
      ID so that the breakpoint code can correctly re-identify the
Simon Marchi Feb. 21, 2018, 1:03 p.m. | #4
On 2018-02-20 22:54, Tom Tromey wrote:
> Tom> Maybe I should avoid this entirely by using std::copy or something 

> like

> Tom> that.

> 

> How about this instead?

> 

> Tom


LGTM.

Patch

diff --git a/gdb/infcall.c b/gdb/infcall.c
index 8b75297251..6ff725a815 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -732,7 +732,6 @@  call_function_by_hand_dummy (struct value *function,
   struct type *ftype = check_typedef (value_type (function));
   CORE_ADDR bp_addr;
   struct frame_id dummy_id;
-  struct cleanup *args_cleanup;
   struct frame_info *frame;
   struct gdbarch *gdbarch;
   struct cleanup *terminate_bp_cleanup;
@@ -1054,21 +1053,18 @@  call_function_by_hand_dummy (struct value *function,
 	}
     }
 
+  std::vector<struct value *> new_args;
   if (hidden_first_param_p)
     {
-      struct value **new_args;
-
       /* Add the new argument to the front of the argument list.  */
-      new_args = XNEWVEC (struct value *, nargs + 1);
+      new_args.reserve (nargs + 1);
+
       new_args[0] = value_from_pointer (lookup_pointer_type (values_type),
 					struct_addr);
       memcpy (&new_args[1], &args[0], sizeof (struct value *) * nargs);
-      args = new_args;
+      args = new_args.data ();
       nargs++;
-      args_cleanup = make_cleanup (xfree, args);
     }
-  else
-    args_cleanup = make_cleanup (null_cleanup, NULL);
 
   /* Create the dummy stack frame.  Pass in the call dummy address as,
      presumably, the ABI code knows where, in the call dummy, the
@@ -1077,8 +1073,6 @@  call_function_by_hand_dummy (struct value *function,
 				bp_addr, nargs, args,
 				sp, struct_return, struct_addr);
 
-  do_cleanups (args_cleanup);
-
   /* Set up a frame ID for the dummy frame so we can pass it to
      set_momentary_breakpoint.  We need to give the breakpoint a frame
      ID so that the breakpoint code can correctly re-identify the