Re: [Qemu-devel] Fwd: qemu code review

[Kevin Wolf <kwolf@redhat.com>]

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 <sgrubb@redhat.com>
>>
>> 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