[PATCH] Fix off-by-one in call to vector::reserve

[PATCH] Fix off-by-one in call to vector::reserve

[Tom Tromey]

While looking at a bug, I noticed what I think is an off-by-one
mistake in a call to vector::reserve.  This code:

      new_args.reserve (args.size ());
      new_args.push_back
	(value_from_pointer (lookup_pointer_type (values_type), struct_addr));
      new_args.insert (new_args.end (), args.begin (), args.end ());

... reserves 'size()' entries, but then proceeds to push one extra
one.

This shouldn't have any really bad effects, as insert will grow the
vector.  Still, it seems better to use the correct size if we're going
to bother calling reserve.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30780
---
 gdb/infcall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/infcall.c b/gdb/infcall.c
index bea5b185ddc..7e19be79a24 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1233,7 +1233,7 @@ call_function_by_hand_dummy (struct value *function,
   if (return_method == return_method_hidden_param)
     {
       /* Add the new argument to the front of the argument list.  */
-      new_args.reserve (args.size ());
+      new_args.reserve (args.size () + 1);
       new_args.push_back
 	(value_from_pointer (lookup_pointer_type (values_type), struct_addr));
       new_args.insert (new_args.end (), args.begin (), args.end ());
-- 
2.40.1

Re: [PATCH] Fix off-by-one in call to vector::reserve

[John Baldwin]

On 8/18/23 7:03 AM, Tom Tromey via Gdb-patches wrote:
> While looking at a bug, I noticed what I think is an off-by-one
> mistake in a call to vector::reserve.  This code:
> 
>        new_args.reserve (args.size ());
>        new_args.push_back
> 	(value_from_pointer (lookup_pointer_type (values_type), struct_addr));
>        new_args.insert (new_args.end (), args.begin (), args.end ());
> 
> ... reserves 'size()' entries, but then proceeds to push one extra
> one.
> 
> This shouldn't have any really bad effects, as insert will grow the
> vector.  Still, it seems better to use the correct size if we're going
> to bother calling reserve.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30780
> ---
>   gdb/infcall.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/infcall.c b/gdb/infcall.c
> index bea5b185ddc..7e19be79a24 100644
> --- a/gdb/infcall.c
> +++ b/gdb/infcall.c
> @@ -1233,7 +1233,7 @@ call_function_by_hand_dummy (struct value *function,
>     if (return_method == return_method_hidden_param)
>       {
>         /* Add the new argument to the front of the argument list.  */
> -      new_args.reserve (args.size ());
> +      new_args.reserve (args.size () + 1);
>         new_args.push_back
>   	(value_from_pointer (lookup_pointer_type (values_type), struct_addr));
>         new_args.insert (new_args.end (), args.begin (), args.end ());

Pedantically speaking I would probably write it as '1 + args.size()' as the
extra pointer is pushed first followed by a copy of all the items in args
(that is, trying to make the expression list the sub-sizes in the same order
they are added to the vector).

Reviewed-by: John Baldwin <jhb@FreeBSD.org>

-- 
John Baldwin

Re: [PATCH] Fix off-by-one in call to vector::reserve

[Tom Tromey]

John> Pedantically speaking I would probably write it as '1 + args.size()' as the
John> extra pointer is pushed first followed by a copy of all the items in args
John> (that is, trying to make the expression list the sub-sizes in the same order
John> they are added to the vector).

Sounds good, I'll do it.

Tom