[PATCH v2 1/3] rtems-debugger: Fixed 32bit pointers

[Joel Sherrill]

On Thu, Mar 18, 2021, 1:10 PM Stephen Clark <stephen.clark at oarcorp.com>
wrote:

> I think Sebastian and Gedare might have referenced this earlier, but I’m
> not sure if how to be that descriptive within the 50 character limit. "Use
> uintptr_t not uint32_t for portability to 64-bit CPUs" is 58 characters
> without a prefix. Even when pared down to something like “Use uintptr_t to
> build on 64-bit CPUs”, there still isn’t room to prepend “rtems-debugger:”.
>

I don't think these HAVE to be less than 80 columns just close. But you
could drop "not uint32_t" and "for portability" if needed

>
>
> *From:* Joel Sherrill <joel at rtems.org>
> *Sent:* Thursday, March 18, 2021 12:50 PM
> *To:* Stephen Clark <stephen.clark at oarcorp.com>
> *Cc:* rtems-devel at rtems.org <devel at rtems.org>
> *Subject:* Re: [PATCH v2 1/3] rtems-debugger: Fixed 32bit pointers
>
>
>
> After picking on Ryan, Alex, and Sebastian, you get it next. :)
>
>
>
> "Fix" or "Fixed" in the short commit message title is not useful
>
> when you browse the log of commits:
>
> https://git.rtems.org/rtems/log/
>
> Something like "Use uint32_t not uintptr_t for portability to 64-bit CPUs"
>
> says a lot more. Think of yourself reading that list of commits and what
>
> messages tell you enough and which leave you thinking that you need
>
> to look at the change to know what was done.
>
>
>
>
>
>
>
> On Thu, Mar 18, 2021 at 12:33 PM Stephen Clark <stephen.clark at oarcorp.com>
> wrote:
>
> Using 32bit types like uint32_t for pointers creates issues on 64 bit
> architectures like AArch64. Replaced occurrences of these with uintptr_t,
> which will work for both 32 and 64 bit architectures.
> ---
>  cpukit/libdebugger/rtems-debugger-server.c | 4 ++--
>  cpukit/libdebugger/rtems-debugger-target.c | 2 +-
>  cpukit/libdebugger/rtems-debugger-target.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/cpukit/libdebugger/rtems-debugger-server.c
> b/cpukit/libdebugger/rtems-debugger-server.c
> index 975ec23a30..f8c485a794 100644
> --- a/cpukit/libdebugger/rtems-debugger-server.c
> +++ b/cpukit/libdebugger/rtems-debugger-server.c
> @@ -1438,7 +1438,7 @@ remote_read_memory(uint8_t* buffer, int size)
>    if (comma == NULL)
>      remote_packet_out_str(r_E01);
>    else {
> -    DB_UINT addr;
> +    uintptr_t addr;
>      DB_UINT length;
>      int     r;
>      addr = hex_decode_uint(&buffer[1]);
> @@ -1468,7 +1468,7 @@ remote_write_memory(uint8_t* buffer, int size)
>    comma = strchr((const char*) buffer, ',');
>    colon = strchr((const char*) buffer, ':');
>    if (comma != NULL && colon != NULL) {
> -    DB_UINT addr;
> +    uintptr_t addr;
>      DB_UINT length;
>      int     r;
>      addr = hex_decode_uint(&buffer[1]);
>
>
>
> I think the changes OK but Chris should comment what
>
> happens on 64-bit address targets.
>
>
>
> I think this may be decoding the gdb protocol message and we need
>
> to know if the field coming in is OK to decode as a uint.
>
>
>
> Your patch is OK but there may be an issue interfacing with the protocol
>
> that this points out.
>
>
>
> diff --git a/cpukit/libdebugger/rtems-debugger-target.c
> b/cpukit/libdebugger/rtems-debugger-target.c
> index bf7579700d..34e4e84d2f 100644
> --- a/cpukit/libdebugger/rtems-debugger-target.c
> +++ b/cpukit/libdebugger/rtems-debugger-target.c
> @@ -168,7 +168,7 @@ rtems_debugger_target_reg_table_size(void)
>  }
>
>  int
> -rtems_debugger_target_swbreak_control(bool insert, DB_UINT addr, DB_UINT
> kind)
> +rtems_debugger_target_swbreak_control(bool insert, uintptr_t addr,
> DB_UINT kind)
>  {
>    rtems_debugger_target*         target = rtems_debugger->target;
>    rtems_debugger_target_swbreak* swbreaks;
> diff --git a/cpukit/libdebugger/rtems-debugger-target.h
> b/cpukit/libdebugger/rtems-debugger-target.h
> index f2abbe5fd3..db356e1f07 100644
> --- a/cpukit/libdebugger/rtems-debugger-target.h
> +++ b/cpukit/libdebugger/rtems-debugger-target.h
> @@ -200,7 +200,7 @@ extern void
> rtems_debugger_target_exception_print(CPU_Exception_frame* frame);
>   * Software breakpoints. These are also referred to as memory breakpoints.
>   */
>  extern int rtems_debugger_target_swbreak_control(bool    insert,
> -                                                 DB_UINT addr,
> +                                                 uintptr_t addr,
>                                                   DB_UINT kind);
>
>  /**
> --
> 2.27.0
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210318/0a5e296f/attachment.html>