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

Christian Biesinger via Gdb-patches
2021-11-01T18:29:47Z

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