From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41528) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axLQE-0007R1-1F for qemu-devel@nongnu.org; Mon, 02 May 2016 17:30:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axLQ1-00052l-OU for qemu-devel@nongnu.org; Mon, 02 May 2016 17:30:24 -0400 References: <1461960516-4717-1-git-send-email-eblake@redhat.com> <1461960516-4717-5-git-send-email-eblake@redhat.com> <20160502153502.GB4882@noname.redhat.com> <5727C3A2.9040307@redhat.com> From: Eric Blake Message-ID: <5727C6C5.4090602@redhat.com> Date: Mon, 2 May 2016 15:29:41 -0600 MIME-Version: 1.0 In-Reply-To: <5727C3A2.9040307@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="sf58RBKwIwO7bIsOV293VLmnFwuIiIiN6" Subject: Re: [Qemu-devel] [PATCH v4 04/14] onenand: Switch to byte-based block access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, "open list:Block layer core" , Max Reitz This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --sf58RBKwIwO7bIsOV293VLmnFwuIiIiN6 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/02/2016 03:16 PM, Eric Blake wrote: >> First of all, why signed? More importantly, though, sec is an int. I'm= >> not sure if we should cast it to uint64_t before shifting (I'm unsure >> because this device seems to supports only sizes that fit in a uint32_= t >> anyway), but if we don't, wouldn't it make things more obvious if offs= et >> were a uint32_t, too? >=20 > Hmm. I guess sec can't be negative, but I didn't check whether sec can > ever be greater than 0x7fffffff/BDRV_SECTOR_SIZE. Depending on that > answer determines whether the shift here overflows - but you are right > that if it CAN overflow 32 bits, then we MUST cast sec to a 64-bit type= > PRIOR to the shift, not just merely assign it to a 64-bit value; and if= > CAN'T overflow, then a 32-bit type is sufficient to hold the answer. > You're also right that unsigned is nicer in general for sizes that > shouldn't be negative. Okay, auditing wasn't as hard as I feared: static int onenand_initfn(SysBusDevice *sbd) =2E.. uint32_t size =3D 1 << (24 + ((s->id.dev >> 4) & 7)); =2E.. s->blocks =3D size >> BLOCK_SHIFT; s->secs =3D size >> 9; so the maximum sec should ever be is 0x80000000 >> 9. I'll stick with uint32_t (since that's what the init function used), and maybe add an assert that we aren't overflowing. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --sf58RBKwIwO7bIsOV293VLmnFwuIiIiN6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXJ8bGAAoJEKeha0olJ0Nqj4UH/1lBHyKMQAsVdh69Tgtok75g okaBodlstJadB7mPgT+Yhju1xBOmyA68JxMhfacKVSfQ6bSss7H0ZRkD51PxU5nZ m/z11QSZfvRwe2FzCjeFb2LAtjZu+i6mNZoysvjDBqXGmTIu3BLCEpXuOBg9OX3b UmhQktMS/mG5vYwt8Ul36QyqwzHg0ptw9YCRIbWFjJGw9bTFmwnKZRVAuOcxarOr MSj/A+U0s3HFF6dHagpsIxJswLgiR2zaotUCXubys7ckPTMtAGW7L+s4w6OH7SeT sbFRMnucTOYpydGbNqyPe5Q500NFvCDl7393+SYxARH2LhqqaxoxNC91W++07HU= =qKcv -----END PGP SIGNATURE----- --sf58RBKwIwO7bIsOV293VLmnFwuIiIiN6--