[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Steve Grubb
Subject: Re: [Qemu-devel] Fwd: qemu code review
Date: Thu, 19 Nov 2009 13:11:56 -0500
User-agent: KMail/1.12.2 (Linux/; KDE/4.3.2; x86_64; ; )

On Thursday 19 November 2009 04:09:48 am Kevin Wolf wrote:
> >> ...
> >> 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?

No. Its likely point out a negative index.

> > Or even better (hopefully the compiler will combine both statements)
> >         memmove(tp->vlan, tp->data, 4);
> >         memmove(tp->data, tp->data + 4, 8);

This would make it happier. But if a comment was made that its intentionally 
overrunning the vlan array, it would cause less concern.
> 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...

Right. Someone might use a cache analysis tool in the future and see that it 
runs faster with reordered fields...


reply via email to

[Prev in Thread] Current Thread [Next in Thread]