From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 6zjECON8OWHRcQAAWB0awg (envelope-from ) for ; Wed, 08 Sep 2021 23:17:55 -0400 Received: by simark.ca (Postfix, from userid 112) id 116E51EE23; Wed, 8 Sep 2021 23:17:55 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3E0B01ECEB for ; Wed, 8 Sep 2021 23:17:54 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7BB35384BC21 for ; Thu, 9 Sep 2021 03:17:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7BB35384BC21 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1631157473; bh=/8aCJTdmDRX6H5u4vDNhdJOomS/hRb7oS2toI9IJlxc=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=oIDf41LPNUl+yXfB0Yl+bTcssMtV/xZbPPok8BNxjze0GN5vqPn78FbrA0CURIPgI d+X6vttPij0flo4QDpICydbCVuhu0ZjStVavGR7Au3yF5hAvlSIaHSHQ+cG9IruEeo vyYrxS0RJWbh04xwmFu5jpUBGpvd1E0JnZ9qqCoc= Received: from smtp.gentoo.org (dev.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) by sourceware.org (Postfix) with ESMTP id 0478E384F02A for ; Thu, 9 Sep 2021 03:17:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0478E384F02A Date: Wed, 8 Sep 2021 23:17:31 -0400 To: Tom de Vries , gdb-patches@sourceware.org, andrew.burgess@embecosm.com Subject: Re: [PATCH] sim: ppc: fix some Wpointer-sign warnings Message-ID: Mail-Followup-To: Tom de Vries , gdb-patches@sourceware.org, andrew.burgess@embecosm.com References: <20210519104646.GA11845@delia> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Y4bm2UpRqhXFgocd" Content-Disposition: inline In-Reply-To: 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: , From: Mike Frysinger via Gdb-patches Reply-To: Mike Frysinger Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" --Y4bm2UpRqhXFgocd Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 19 May 2021 12:32, Mike Frysinger via Gdb-patches wrote: > On 19 May 2021 12:46, Tom de Vries wrote: > > --- a/sim/ppc/hw_memory.c > > +++ b/sim/ppc/hw_memory.c > > @@ -199,9 +199,9 @@ hw_memory_init_address(device *me) > > cell_nr +=3D 2) { > > hw_memory_chunk *new_chunk =3D ZALLOC(hw_memory_chunk); > > device_find_integer_array_property(me, "available", cell_nr, > > - &new_chunk->address); > > + (signed_cell *)&new_chunk->address); > > device_find_integer_array_property(me, "available", cell_nr + 1, > > - &new_chunk->size); > > + (signed_cell *)&new_chunk->size); > >=20 > > --- a/sim/ppc/hw_opic.c > > +++ b/sim/ppc/hw_opic.c > > @@ -417,10 +417,12 @@ hw_opic_init_data(device *me) > > } > > if (!device_find_integer_array_property(me, "interrupt-ranges", > > reg_nr * 2, > > - &opic->isu_block[isb].int_number) > > + (signed_cell *) > > + &opic->isu_block[isb].int_number) > > || !device_find_integer_array_property(me, "interrupt-ranges", > > reg_nr * 2 + 1, > > - &opic->isu_block[isb].range)) > > + (signed_cell *) > > + &opic->isu_block[isb].range)) >=20 > these ones i'm not sure about. it does fix the warnings, and it doesn't > change the status quo behavior, but i don't think it's the actual fix we > would want. if the device tree has a negative number, it'll get converted > to an unsigned number. i haven't thought hard as to what the right fix > would look like here. >=20 > i think we'd have to look at what other device tree users are doing like > in boot loaders (e.g. u-boot) and in the linux kernel. i poked this one a bit today. i think your cast is OK here. feel free to correct anything below that i get wrong. the dtc/libfdt code internally processes the fdt blob as a series of bytes without any type information. typing is left to the caller. in the libfdt code, they have core APIs that read/write bytes, and a few helper functions to cast/convert those bytes to the right value (e.g. a u32). in the ppc sim code, the core APIs use signed integers, and then the callers convert to unsigned, usually implicitly (e.g. the device_find_integer_property call is assigned to an unsigned variable _a lot_). it seems like the code just doesn't happen to use unsigned & arrays a lot, so that's why there's only these few places that tickle the signed warning errors. we could add some core APIs to the ppc sim that deal with raw bytes and then add some helpers to convert to the right type, but that seems like a lot of lifting for what boils down to your cast here. and i think a lot of the ppc code should either get converted to existing sim common code, or we should stand up proper APIs in the common code first, or use standard libraries to do all the processing (e.g. libfdt). either way, this device.c code would all get deleted, and callers (like these hw_*.c files) would get converted. i'll merge your fix with a bit more detail, and enable the warning too. -mike --Y4bm2UpRqhXFgocd Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEuQK1JxMl+JKsJRrUQWM7n+g39YEFAmE5fMsACgkQQWM7n+g3 9YHdbg//TbrV+1W/qUNic6G/QXRzQxUvheLMHw1nem/960hElihbCyS8TuoujEcx TIsNLIK0aGGfZuMFqmXLgVY6TXYwDM+y+6k/C6B5BIE3oj3eXhvd05cXx7XYduBN /CMJUE1Gm+rNfQqMUNzL8R1C8xaKV0Iycsie9bfGcmSbGNUElA+V1mAa4b33O/gN c08hPno4EnILVSFA8NhMHL5fcjqqEVz+fPbueNph4+SMS028qjqlV1X+77FBtDFW o7KP9AZw/4lPLKkmHpsaIDJbyUhKOFIFv1F6TeLOIOwGdwIYgyybKmsegLcJaJ3l AuYS6DbA8EMnzSM4N3C6g+C9q0P+hn7MK5l/OjPBDCATV0/X+wmeER207nFgdk+o 11cIAv4cGcRW+BVZVsstU6wEWAfLtqn/kRuTqmnan5LA/qdjcNNFZNAjFkv2LEWK 2Z6jP9OfVV1BzJWeT73KqHtox6mz0a2IgpG+4zjfc7T6MP2LZA/XsKAMuH6G4N0Z hwqJe0qcIDhkDeAPE7PU9PZJErMrelTZtS4lkBo2AxEG6/iPLEnFzllDfdMahGdt LhQJoXeHKSoamctNMwrN+2Ig/OxQna0VBqPWigLcuttC+wBKKpZKv8h9rHerV87C HEkWAVVL4ZKvqxKwO+RoWrBIr2amLEmjh4Gzsa8QaFBLewVQix8= =Yhpx -----END PGP SIGNATURE----- --Y4bm2UpRqhXFgocd--