From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGXln-0007Y9-Sa for qemu-devel@nongnu.org; Tue, 05 Jan 2016 14:59:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGXli-0000nk-Ty for qemu-devel@nongnu.org; Tue, 05 Jan 2016 14:59:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48662) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGXli-0000nZ-MW for qemu-devel@nongnu.org; Tue, 05 Jan 2016 14:59:46 -0500 References: <1452015002-28493-2-git-send-email-clg@fr.ibm.com> From: Eric Blake Message-ID: <568C20AC.3020805@redhat.com> Date: Tue, 5 Jan 2016 12:59:40 -0700 MIME-Version: 1.0 In-Reply-To: <1452015002-28493-2-git-send-email-clg@fr.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="q6QPEc9Ac8POsIBpd8WoEQ3iBPOI3FDW5" Subject: Re: [Qemu-devel] [PATCH 1/8] ipmi: fix SDR length value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= , Corey Minyard Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --q6QPEc9Ac8POsIBpd8WoEQ3iBPOI3FDW5 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/05/2016 10:29 AM, C=C3=A9dric Le Goater wrote: [meta-comment] Your messages were not marked in-reply-to: the 0/8 cover letter, but came through as separate threads. This makes it harder to follow, especially in mail clients that sort top-level threads by most recent activity on the thread. > The IPMI BMC simulator populates the SDR table with a set of initial > SDRs. The length of each SDR is taken from the record itself (byte 4) > which does not include the size of the header. But, the full length > (header + data) is required by the sdr_add_entry() routine. >=20 > Signed-off-by: C=C3=A9dric Le Goater > --- >=20 > Maybe we could use a sdr struct/typedef to clarify the code. See > patch 7: "ipmi: introduce an ipmi_bmc_init_sensor() API" >=20 > hw/ipmi/ipmi_bmc_sim.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) >=20 > diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c > index 0a59e539f549..559e1398d669 100644 > --- a/hw/ipmi/ipmi_bmc_sim.c > +++ b/hw/ipmi/ipmi_bmc_sim.c > @@ -362,7 +362,7 @@ static int sdr_find_entry(IPMISdr *sdr, uint16_t re= cid, > =20 > while (pos < sdr->next_free) { > uint16_t trec =3D sdr->sdr[pos] | (sdr->sdr[pos + 1] << 8); > - unsigned int nextpos =3D pos + sdr->sdr[pos + 4]; > + unsigned int nextpos =3D pos + sdr->sdr[pos + 4] + 5; 5 feels like a magic number; should you use a #define and name it? > @@ -1709,20 +1709,20 @@ static void ipmi_sim_init(Object *obj) > for (i =3D 0;;) { > int len; > if ((i + 5) > sizeof(init_sdrs)) { > - error_report("Problem with recid 0x%4.4x: \n", i); > + error_report("Problem with recid 0x%4.4x\n", i); Please drop the trailing \n as long as you are touching this. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --q6QPEc9Ac8POsIBpd8WoEQ3iBPOI3FDW5 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/ iQEcBAEBCAAGBQJWjCCsAAoJEKeha0olJ0Nq20oH/jkauHxZx/kk3QgYCowTms9N JoeSywlaP0ZsxfQ38EFh6iVtaQWELXu57Bo4CxKN8l6BuOUVoGsQBSX+PjD8eHRE UNoWpewHYrvjoIJyBsP3FQ1de+QhK31aGR1Bew3Z8u5ZDNKL5/UZQo3KEcAdloDa tXes2JioL5hx0i/kO25Cyu3YI+xWBaruBT9vdYN8f2QQepD4HDRDCrfhMvsu5Izy pVBBn/6MWfVoobpoeA0vB+mXRdn5i62PgYB8lLAeoF1jh3H93sb+E93f0MAUBQ4r 1eADmbHZOhybGa0FVztUjajwt1ztWI+KKdm40ER/zKMRa7a56tiJrZsZTsC2lYs= =jtEB -----END PGP SIGNATURE----- --q6QPEc9Ac8POsIBpd8WoEQ3iBPOI3FDW5--