On Thu, Mar 18, 2021 at 12:14 PM Joel Sherrill <j...@rtems.org> wrote: > > > > On Thu, Mar 18, 2021, 1:10 PM Stephen Clark <stephen.cl...@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
In the first line of the commit, we want to adhere to about 50 characters. Otherwise, the short-log and email subject lines get excessive. I think the commit message he used is fine, it gives sufficient information about *what* has been fixed *where*, while the details can be gotten by drilling down another level. Brevity in the first line is appreciated. I'm sure we had written this down somewhere... >> >> >> >> From: Joel Sherrill <j...@rtems.org> >> Sent: Thursday, March 18, 2021 12:50 PM >> To: Stephen Clark <stephen.cl...@oarcorp.com> >> Cc: rtems-de...@rtems.org <devel@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.cl...@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@rtems.org >> http://lists.rtems.org/mailman/listinfo/devel > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel