Re: [PATCH] sim: mn10300: clean up pointer casts

[Christian Biesinger <cbiesinger@google.com>]

On Mon, Nov 1, 2021 at 12:22 PM Mike Frysinger <vapier@gentoo.org> wrote:
>
> On 01 Nov 2021 12:07, Christian Biesinger wrote:
> > On Sun, Oct 31, 2021 at 11:45 PM Mike Frysinger via Gdb-patches
> > <gdb-patches@sourceware.org> wrote:
> > > --- a/sim/mn10300/dv-mn103ser.c
> > > +++ b/sim/mn10300/dv-mn103ser.c
> > > @@ -238,7 +238,7 @@ do_polling_event (struct hw *me,
> > >  {
> > >    SIM_DESC sd = hw_system (me);
> > >    struct mn103ser *serial = hw_data(me);
> > > -  long serial_reg = (long) data;
> > > +  long serial_reg = (uintptr_t) data;
> >
> > Shouldn't you change the variable type too?
>
> the types are already confused, but long here is fine as any other.  uintptr_t
> is not really any more correct.  serial_reg comes in as an unsigned_word via
> the memory read/write callbacks, (encoded) cast to void*, and (decoded) cast
> to long.  the memory read/write callbacks verify the values are always within
> the serial_register_types enum range which looks like [0,16] range, so any of
> these types work.
>
> > Otherwise this patch seems to be somewhat pointless?
> >     if ( nr_bytes == 1 )
>
> i don't know which one you mean, or why you think any of them are pointless.
> the nr_bytes checks are in the read/write code paths of the memory mapped
> register callbacks.  so nr_bytes is needed & correct there.

I forgot to delete that nr_bytes line; the pointless comment was about
casting to uintprtr_t but still storing the result in a long. Why not
keep it as a cast to long then?

Christian