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 > > > *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
