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:”.
From: Joel Sherrill <[email protected]> Sent: Thursday, March 18, 2021 12:50 PM To: Stephen Clark <[email protected]> Cc: [email protected] <[email protected]> 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 <[email protected]<mailto:[email protected]>> 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 [email protected]<mailto:[email protected]> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________ devel mailing list [email protected] http://lists.rtems.org/mailman/listinfo/devel
