On 01 Nov 2021 14:28, Christian Biesinger via Gdb-patches wrote:
> 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?

that was the point of the patch in the first place :).  i prob should have
included the compiler errors in the log, but basically any target where the
sizeof(long) != sizeof(void*) will generate an error.  LLP64 systems is where
you'll find this, and most notably, 64-bit Windows does it.
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

since sizeof(uintptr_t) is guaranteed to equal sizeof(void*), it's the "right"
way to convert integers to/from pointers.  in this case, the event framework
is using void* as an opaque storage, and this particular model is (ab)using
that to pass values rather than pointers to larger storage.
-mike