FYI: [Ada] Rework ada_value_primitive_packed_val

  Hi Joel,

On 10/09/2015 10:41 PM, Joel Brobecker wrote:

> FYI, I have just pushed a set of commits we made a while ago at AdaCore
> after I realized ada_value_primitive_packed_val could be written in
> a way that makes it easier to understand.

I noticed that this series regressed the C++ build:

../../src/gdb/ada-lang.c: In function ‘value* ada_value_primitive_packed_val(value*, const gdb_byte*, long int, int, int, type*)’:
../../src/gdb/ada-lang.c:2553:36: error: invalid conversion from ‘void*’ to ‘gdb_byte* {aka unsigned char*}’ [-fpermissive]
       staging = malloc (staging_len);
                                    ^

(similarly for alloca.)

Looking at the code, I noticed the newly added casts, and thought of giving
it a try at eliminating them.  See patches below.  WDYT?
  

> Looking at the code, I noticed the newly added casts, and thought of giving
> it a try at eliminating them.  See patches below.  WDYT?

They look good, thanks for doing that!

> >From b58eef432cc9705a3e95d2cf91b0f64ae102f2e1 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Sat, 10 Oct 2015 14:31:41 +0100
> Subject: [PATCH 1/2] ada-lang.c: malloc/alloca casts for C++
> 
> gdb/ChangeLog:
> 2015-10-10  Pedro Alves  <palves@redhat.com>
> 
> 	* ada-lang.c (ada_value_primitive_packed_val): Add casts to malloc
> 	and alloca calls.
> ---
>  gdb/ada-lang.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 97f0c49..6d1998a 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -2550,7 +2550,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>  	 we do, is unpack the data into a byte-aligned buffer, and then
>  	 use that buffer as our object's value for resolving the type.  */
>        staging_len = (bit_size + HOST_CHAR_BIT - 1) / HOST_CHAR_BIT;
> -      staging = malloc (staging_len);
> +      staging = (gdb_byte *) malloc (staging_len);
>        make_cleanup (xfree, staging);
>  
>        ada_unpack_from_contents (src, bit_offset, bit_size,
> @@ -2581,7 +2581,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>        int src_len = (bit_size + bit_offset + HOST_CHAR_BIT - 1) / 8;
>  
>        v = value_at (type, value_address (obj) + offset);
> -      src = alloca (src_len);
> +      src = (gdb_byte *) alloca (src_len);
>        read_memory (value_address (v), src, src_len);
>      }
>    else
> -- 
> 1.9.3
> 

> >From be61a6e02fc3dce50f4ce16faa3b8a458d2a451a Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Sat, 10 Oct 2015 13:53:47 +0100
> Subject: [PATCH 2/2] ada-lang.c:ada_value_primitive_packed_val: const
>  correctness
> 
> gdb/ChangeLog:
> 2015-10-10  Pedro Alves  <palves@redhat.com>
> 
> 	* ada-lang.c (ada_value_primitive_packed_val): Constify
> 	locals.  Use value_contents_writeable.  Remove casts.
> ---
>  gdb/ada-lang.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 6d1998a..383db99 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -2525,7 +2525,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>                                  struct type *type)
>  {
>    struct value *v;
> -  gdb_byte *src;                /* First byte containing data to unpack */
> +  const gdb_byte *src;                /* First byte containing data to unpack */
>    gdb_byte *unpacked;
>    const int is_scalar = is_scalar_type (type);
>    const int is_big_endian = gdbarch_bits_big_endian (get_type_arch (type));
> @@ -2536,9 +2536,9 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>    type = ada_check_typedef (type);
>  
>    if (obj == NULL)
> -    src = (gdb_byte *) valaddr + offset;
> +    src = valaddr + offset;
>    else
> -    src = (gdb_byte *) value_contents (obj) + offset;
> +    src = value_contents (obj) + offset;
>  
>    if (is_dynamic_type (type))
>      {
> @@ -2574,20 +2574,22 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>    if (obj == NULL)
>      {
>        v = allocate_value (type);
> -      src = (gdb_byte *) valaddr + offset;
> +      src = valaddr + offset;
>      }
>    else if (VALUE_LVAL (obj) == lval_memory && value_lazy (obj))
>      {
>        int src_len = (bit_size + bit_offset + HOST_CHAR_BIT - 1) / 8;
> +      gdb_byte *buf;
>  
>        v = value_at (type, value_address (obj) + offset);
> -      src = (gdb_byte *) alloca (src_len);
> -      read_memory (value_address (v), src, src_len);
> +      buf = (gdb_byte *) alloca (src_len);
> +      read_memory (value_address (v), buf, src_len);
> +      src = buf;
>      }
>    else
>      {
>        v = allocate_value (type);
> -      src = (gdb_byte *) value_contents (obj) + offset;
> +      src = value_contents (obj) + offset;
>      }
>  
>    if (obj != NULL)
> @@ -2610,7 +2612,7 @@ ada_value_primitive_packed_val (struct value *obj, const gdb_byte *valaddr,
>      }
>    else
>      set_value_bitsize (v, bit_size);
> -  unpacked = (gdb_byte *) value_contents (v);
> +  unpacked = value_contents_writeable (v);
>  
>    if (bit_size == 0)
>      {
> -- 
> 1.9.3
>
  

On 10/12/2015 04:56 PM, Joel Brobecker wrote:
>> Looking at the code, I noticed the newly added casts, and thought of giving
>> it a try at eliminating them.  See patches below.  WDYT?
> 
> They look good, thanks for doing that!

Pushed, thanks.