| Message ID | 1258656295-5688-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | New |
| Headers | show |
Stefan Weil wrote: > A code review run by Steve Grubb complained about code in e1000.c: > > 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. > + /* Fields vlan and data must not be reordered or separated. */ > unsigned char vlan[4]; > unsigned char data[0x10000]; Wouldnt it be better to stuff both into a struct or something? I guess from the '12' that the data size can vary, but thats less important if they are packed in a way that the compiler (and coders!) know not to seperate them. -Ian
diff --git a/hw/e1000.c b/hw/e1000.c index 00f6a57..6f9adb5 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -89,6 +89,7 @@ typedef struct E1000State_st { struct e1000_tx { unsigned char header[256]; unsigned char vlan_header[4]; + /* Fields vlan and data must not be reordered or separated. */ unsigned char vlan[4]; unsigned char data[0x10000]; uint16_t size; @@ -383,7 +384,8 @@ xmit_seg(E1000State *s) if (tp->sum_needed & E1000_TXD_POPTS_IXSM) putsum(tp->data, tp->size, tp->ipcso, tp->ipcss, tp->ipcse); if (tp->vlan_needed) { - memmove(tp->vlan, tp->data, 12); + memmove(tp->vlan, tp->data, 4); + memmove(tp->data, tp->data + 4, 8); memcpy(tp->data + 8, tp->vlan_header, 4); qemu_send_packet(s->vc, tp->vlan, tp->size + 4); } else
A code review run by Steve Grubb complained about code in e1000.c: 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. This was fixed by splitting the memmove in two calls and adding a comment to the declaration of vlan and data. Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- hw/e1000.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)