[c++,1/5] lm32: Add (const gdb_byte *) cast

Message ID 1444538238-7468-1-git-send-email-simon.marchi@polymtl.ca
State New, archived
Headers

Commit Message

Simon Marchi Oct. 11, 2015, 4:37 a.m. UTC
  There might be a cleaner sequence of function calls to do what is done
here (I don't know), but since I don't want to risk breaking anything,
it's safer to just add the required cast.

gdb/ChangeLog:

	* lm32-tdep.c (lm32_push_dummy_call): Add cast.
---
 gdb/lm32-tdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Yao Qi Oct. 12, 2015, 10:51 a.m. UTC | #1
Simon Marchi <simon.marchi@polymtl.ca> writes:

> There might be a cleaner sequence of function calls to do what is done
> here (I don't know), but since I don't want to risk breaking anything,
> it's safer to just add the required cast.
>
> gdb/ChangeLog:
>
> 	* lm32-tdep.c (lm32_push_dummy_call): Add cast.

Patch is OK.
  
Pedro Alves Oct. 12, 2015, 11:10 a.m. UTC | #2
On 10/11/2015 05:37 AM, Simon Marchi wrote:
> There might be a cleaner sequence of function calls to do what is done
> here (I don't know), but since I don't want to risk breaking anything,
> it's safer to just add the required cast.
> 
> gdb/ChangeLog:
> 
> 	* lm32-tdep.c (lm32_push_dummy_call): Add cast.
> ---
>  gdb/lm32-tdep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
> index 25a7e1e..a0defad 100644
> --- a/gdb/lm32-tdep.c
> +++ b/gdb/lm32-tdep.c
> @@ -289,7 +289,7 @@ lm32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  	regcache_cooked_write_unsigned (regcache, first_arg_reg + i, val);
>        else
>  	{
> -	  write_memory (sp, (void *) &val, TYPE_LENGTH (arg_type));
> +	  write_memory (sp, (const gdb_byte *) &val, TYPE_LENGTH (arg_type));

This reveals that the code has a host-dependency.  It is assuming the
byte order of the target is the same as host's.

Please replace this with a call to write_memory_unsigned_integer.

Looks like this port hasn't been touched ever since it was
originally contributed.  Jon, is there still interest in this port?

Thanks,
Pedro Alves
  
Jon Beniston Oct. 12, 2015, 12:18 p.m. UTC | #3
Hi Pedro,

Yes, I believe people are still using this port. (Probably no one on a host
with different byte ordering though!)

Regards,
Jon

-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: 12 October 2015 12:11
To: Simon Marchi; gdb-patches@sourceware.org
Cc: Jon Beniston
Subject: Re: [PATCH c++ 1/5] lm32: Add (const gdb_byte *) cast

On 10/11/2015 05:37 AM, Simon Marchi wrote:
> There might be a cleaner sequence of function calls to do what is done 
> here (I don't know), but since I don't want to risk breaking anything, 
> it's safer to just add the required cast.
> 
> gdb/ChangeLog:
> 
> 	* lm32-tdep.c (lm32_push_dummy_call): Add cast.
> ---
>  gdb/lm32-tdep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c index 25a7e1e..a0defad 
> 100644
> --- a/gdb/lm32-tdep.c
> +++ b/gdb/lm32-tdep.c
> @@ -289,7 +289,7 @@ lm32_push_dummy_call (struct gdbarch *gdbarch, struct
value *function,
>  	regcache_cooked_write_unsigned (regcache, first_arg_reg + i, val);
>        else
>  	{
> -	  write_memory (sp, (void *) &val, TYPE_LENGTH (arg_type));
> +	  write_memory (sp, (const gdb_byte *) &val, TYPE_LENGTH 
> +(arg_type));

This reveals that the code has a host-dependency.  It is assuming the byte
order of the target is the same as host's.

Please replace this with a call to write_memory_unsigned_integer.

Looks like this port hasn't been touched ever since it was originally
contributed.  Jon, is there still interest in this port?

Thanks,
Pedro Alves
  
Simon Marchi Oct. 21, 2015, 6:44 p.m. UTC | #4
On 15-10-11 12:37 AM, Simon Marchi wrote:
> There might be a cleaner sequence of function calls to do what is done
> here (I don't know), but since I don't want to risk breaking anything,
> it's safer to just add the required cast.
> 
> gdb/ChangeLog:
> 
> 	* lm32-tdep.c (lm32_push_dummy_call): Add cast.
> ---
>  gdb/lm32-tdep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
> index 25a7e1e..a0defad 100644
> --- a/gdb/lm32-tdep.c
> +++ b/gdb/lm32-tdep.c
> @@ -289,7 +289,7 @@ lm32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>  	regcache_cooked_write_unsigned (regcache, first_arg_reg + i, val);
>        else
>  	{
> -	  write_memory (sp, (void *) &val, TYPE_LENGTH (arg_type));
> +	  write_memory (sp, (const gdb_byte *) &val, TYPE_LENGTH (arg_type));
>  	  sp -= 4;
>  	}
>      }
> 

Ping for patches 1 and 4.
  
Pedro Alves Oct. 22, 2015, 10:55 a.m. UTC | #5
On 10/21/2015 07:44 PM, Simon Marchi wrote:

> Ping for patches 1 and 4.

I think patch #1 is already in, it was the one that ended up
using write_memory_unsigned_integer.

Thanks,
Pedro Alves
  
Simon Marchi Oct. 22, 2015, 1:13 p.m. UTC | #6
On 15-10-22 06:55 AM, Pedro Alves wrote:
> On 10/21/2015 07:44 PM, Simon Marchi wrote:
> 
>> Ping for patches 1 and 4.
> 
> I think patch #1 is already in, it was the one that ended up
> using write_memory_unsigned_integer.
> 
> Thanks,
> Pedro Alves
> 

Ah indeed, sorry about that.
  
Simon Marchi Oct. 22, 2015, 1:51 p.m. UTC | #7
On 15-10-21 02:44 PM, Simon Marchi wrote:
> Ping for patches 1 and 4.

This is now all pushed, except #5 which is dropped because
Pedro has a more appropriate fix.
  

Patch

diff --git a/gdb/lm32-tdep.c b/gdb/lm32-tdep.c
index 25a7e1e..a0defad 100644
--- a/gdb/lm32-tdep.c
+++ b/gdb/lm32-tdep.c
@@ -289,7 +289,7 @@  lm32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	regcache_cooked_write_unsigned (regcache, first_arg_reg + i, val);
       else
 	{
-	  write_memory (sp, (void *) &val, TYPE_LENGTH (arg_type));
+	  write_memory (sp, (const gdb_byte *) &val, TYPE_LENGTH (arg_type));
 	  sp -= 4;
 	}
     }