There is a Coverity issue in gmac_try_send_next_packet() relating to it setting a variable to a value that is always overwritten. While I was looking at that, I noticed a number of oddities in this function. This patchseries collects those up. Note that I have only tested this with 'make check' and 'make check-functional', and since I couldn't find a datasheet I have some questions about the hardware behaviour. Review and testing would be appreciated.
The first patch is about something I noticed last year, but never got a reply on: https://lore.kernel.org/qemu-devel/CAFEAcA8Jx=CrXCSzOtjtEky5ikvtatk8N2gz=nkccpwewo1...@mail.gmail.com/ The code appears to mishandle the case where the function ends up sending more than one packet, such that the second packet gets sent with the contents of the first one. When that bug has been fixed, the length and prev_buf_size variables become exact copies of each other, so patch 2 gets rid of the unnecessary 'length' variable. Here I have a question about the hardware behaviour: what does it do if the guest attempts to assemble a very large packet ? In this patch I retain the current behaviour ("wrap the length at 64K and send a short packet"), but this seems to me unlikely to be correct. Should we be detecting the huge packet and flagging an error somewhere? The third patch fixes a classic C thinko where we do sizeof(some pointer) when we meant to check against the size of the dynamically allocated buffer. The fourth patch fixes the Coverity error by dropping the not-very-useful 'buf' local variable entirely. thanks -- PMM Peter Maydell (4): hw/net/npcm_gmac.c: Send the right data for second packet in a row hw/net/npcm_gmac.c: Unify length and prev_buf_size variables hw/net/npcm_gmac.c: Correct test for when to reallocate packet buffer hw/net/npcm_gmac.c: Drop 'buf' local variable hw/net/npcm_gmac.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) -- 2.43.0
