From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NB32e-0005Lr-SI for qemu-devel@nongnu.org; Thu, 19 Nov 2009 04:11:04 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NB32Z-0005Gp-LA for qemu-devel@nongnu.org; Thu, 19 Nov 2009 04:11:03 -0500 Received: from [199.232.76.173] (port=41266 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NB32Z-0005Gg-DZ for qemu-devel@nongnu.org; Thu, 19 Nov 2009 04:10:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:14675) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NB32Y-0006YF-Ta for qemu-devel@nongnu.org; Thu, 19 Nov 2009 04:10:59 -0500 Message-ID: <4B050B5C.4080908@redhat.com> Date: Thu, 19 Nov 2009 10:09:48 +0100 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] Fwd: qemu code review References: <4B03DD07.7090300@redhat.com> <4B0445B1.1080207@mail.berlios.de> In-Reply-To: <4B0445B1.1080207@mail.berlios.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: sgrubb@redhat.com, "qemu-devel@nongnu.org" Am 18.11.2009 20:06, schrieb Stefan Weil: > Kevin Wolf schrieb: >> Hi all, >> >> as Steve suggests, I'm forwarding the list of issues he found to the >> mailing list. I've already looked at a few points in the block code and >> sent patches. If everyone picks up one point, we should get through the >> list quickly. Who volunteers for the TCG ones? ;-) >> >> Kevin >> >> -------- Original-Nachricht -------- >> Betreff: [virt-devel] qemu code review >> Datum: Tue, 17 Nov 2009 14:05:33 -0500 >> Von: Steve Grubb >> >> Hello, >> >> I took a few hours to run qemu through an analysis tool. Below are the >> results >> of checking everything. I don't interact with the qemu community and >> thought >> someone here might want to take these finding upstream. The review was >> against >> 0.11.0-11 in rawhide. >> >> Thanks, >> -Steve >> >> ----------------------------- >> >> ... >> In hw/e1000.c at line 89, vlan is declared to be 4 bytes. At line 382 is an >> attempt to do a memmove over it with a size of 12. >> > > Obviously this was intentional. Would replacing > memmove(tp->vlan, tp->data, 12); > by > memmove(tp->data - 4, tp->data, 12); > be better and satisfy the analysis tool? Or even better > (hopefully the compiler will combine both statements) > memmove(tp->vlan, tp->data, 4); > memmove(tp->data, tp->data + 4, 8); I don't even know which analysis tool he used. Steve? But I think splitting it into two memmoves is better anyway. There is no warning in the declaration of the struct that these fields need to be consecutive, so someone might have the idea of reordering the fields or inserting a new one in between and things break... Kevin