From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) by sourceware.org (Postfix) with ESMTP id 9A687385800A for ; Tue, 2 Nov 2021 04:18:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9A687385800A Received: by smtp.gentoo.org (Postfix, from userid 559) id 2303F342DB1; Tue, 2 Nov 2021 04:18:57 +0000 (UTC) Date: Tue, 2 Nov 2021 00:19:00 -0400 From: Mike Frysinger To: Christian Biesinger Cc: Christian Biesinger via Gdb-patches Subject: Re: [PATCH] sim: mn10300: clean up pointer casts Message-ID: Mail-Followup-To: Christian Biesinger , Christian Biesinger via Gdb-patches References: <20211101034519.29154-1-vapier@gentoo.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="YvLRbfr9ZgE3aQNi" Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Nov 2021 04:19:00 -0000 --YvLRbfr9ZgE3aQNi Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 01 Nov 2021 15:35, Christian Biesinger via Gdb-patches wrote: > On Mon, Nov 1, 2021 at 2:59 PM Mike Frysinger wrote: > > > > On 01 Nov 2021 14:28, Christian Biesinger via Gdb-patches wrote: > > > On Mon, Nov 1, 2021 at 12:22 PM Mike Frysinger wr= ote: > > > > On 01 Nov 2021 12:07, Christian Biesinger wrote: > > > > > On Sun, Oct 31, 2021 at 11:45 PM Mike Frysinger via Gdb-patches > > > > > 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 =3D hw_system (me); > > > > > > struct mn103ser *serial =3D hw_data(me); > > > > > > - long serial_reg =3D (long) data; > > > > > > + long serial_reg =3D (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 (deco= ded) cast > > > > to long. the memory read/write callbacks verify the values are alw= ays 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 =3D=3D 1 ) > > > > > > > > i don't know which one you mean, or why you think any of them are p= ointless. > > > > 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 h= ave > > included the compiler errors in the log, but basically any target where= the > > sizeof(long) !=3D 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-i= nt-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 fram= ework > > is using void* as an opaque storage, and this particular model is (ab)u= sing > > that to pass values rather than pointers to larger storage. >=20 > I see, so what the patch does is to cast to uintptr_t to avoid the > compile error about losing precision and rely on an implicit > conversion to long? right > My suggestion would be to make that a bit clearer in the commit sure, sorry about that. i'll try to include more details in the future. > message and perhaps explicitly convert from uintptr_t to long. eh -- see above about my investment interest in these models :). i don't think implicit conversions between integer types are a bad thing to avoid in general, especially given the narrow scope. -mike --YvLRbfr9ZgE3aQNi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEuQK1JxMl+JKsJRrUQWM7n+g39YEFAmGAvDMACgkQQWM7n+g3 9YGoRQ//cCiKj3eDSpC/xiLkh7xhxo5yOpEFiZ89qnMMgIpicWW6RUtvMRC3kZO4 2FufHdgHic+zj7XWDppPXfdLg3Jicir6SJ7nBMy0nIAJUvqMGITy9jzajjYzPl8m aVqguqX66g/9Hec/ll7tTOrPKOvZw9OcNLj+VrOVjoeO8Zlkyldbv7Mp2xU8LLWh m97PGJSJH6+EvhhpRMtRMxiKRKL/CEOvBy1r6HGX4zqxw526/WM0cPgryL/3Pp8+ 0BgWDCVVSYhaKqLWAM1dkNySa8kCmWcv7EpC7PnfgYf23MTCQ0GvqGxqG6jCm2// lL6A030niQT3A4T2BbjaOXj7MISKwHLNwKSx6tU1cyHUxdagJvGbf0Xkn371UeA0 q36OqJ6LEw8SnUSsYs+OLVuxkonNkosQJzinGQyoH4L+PLzRVUH7qNjZLM6Xnjud 4dF0HiKERKPaXfc7oZUFPYMdQgRGw6i26IXRqtpPMk6/3cofM4l5L4jtgI4m32+Q dSm4im5XvV2uTggmIdZwuucRxCueiIx4fpq6Z4k3enbEz/caTvdAcQv3404A+GOa NBqS6HdhKOWZAwLaMciSooHx0+lqXHSRO1RBD+vjmvUhccTKi9gETvJBQ/Evckn4 ClUIpV933cyCQxPojemJbNhL5eJgrubo6DE+FLViWm3wHhsn1jw= =u5lm -----END PGP SIGNATURE----- --YvLRbfr9ZgE3aQNi--