In gmac_try_send_next_packet() we have code that does "if this block
of data won't fit in the buffer, reallocate it".  However, the
condition it uses is
  if ((prev_buf_size + tx_buf_len) > sizeof(buf))
where buf is a uint8_t *.

This means that sizeof(buf) is always 8 bytes, and the condition will
almost always be true, so we will reallocate the buffer more often
than we need to.

Correct the condition to test against tx_buffer_size, which is
where we track how big the allocated buffer is.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/npcm_gmac.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/npcm_gmac.c b/hw/net/npcm_gmac.c
index a0050a7725f..0c17ae9b2a9 100644
--- a/hw/net/npcm_gmac.c
+++ b/hw/net/npcm_gmac.c
@@ -569,7 +569,7 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
         tx_buf_len = TX_DESC_TDES1_BFFR1_SZ_MASK(tx_desc.tdes1);
         buf = &tx_send_buffer[prev_buf_size];
 
-        if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+        if ((prev_buf_size + tx_buf_len) > tx_buffer_size) {
             tx_buffer_size = prev_buf_size + tx_buf_len;
             tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
             buf = &tx_send_buffer[prev_buf_size];
@@ -591,7 +591,7 @@ static void gmac_try_send_next_packet(NPCMGMACState *gmac)
             tx_buf_len = TX_DESC_TDES1_BFFR2_SZ_MASK(tx_desc.tdes1);
             buf = &tx_send_buffer[prev_buf_size];
 
-            if ((prev_buf_size + tx_buf_len) > sizeof(buf)) {
+            if ((prev_buf_size + tx_buf_len) > tx_buffer_size) {
                 tx_buffer_size = prev_buf_size + tx_buf_len;
                 tx_send_buffer = g_realloc(tx_send_buffer, tx_buffer_size);
                 buf = &tx_send_buffer[prev_buf_size];
-- 
2.43.0

On 14/7/25 18:55, Peter Maydell wrote:
> In gmac_try_send_next_packet() we have code that does "if this block
> of data won't fit in the buffer, reallocate it".  However, the
> condition it uses is
>    if ((prev_buf_size + tx_buf_len) > sizeof(buf))
> where buf is a uint8_t *.
> 
> This means that sizeof(buf) is always 8 bytes, and the condition will
> almost always be true, so we will reallocate the buffer more often
> than we need to.
> 
> Correct the condition to test against tx_buffer_size, which is
> where we track how big the allocated buffer is.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/net/npcm_gmac.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thank you for catching that!

On Mon, Jul 14, 2025 at 1:08 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 14/7/25 18:55, Peter Maydell wrote:
> > In gmac_try_send_next_packet() we have code that does "if this block
> > of data won't fit in the buffer, reallocate it".  However, the
> > condition it uses is
> >    if ((prev_buf_size + tx_buf_len) > sizeof(buf))
> > where buf is a uint8_t *.
> >
> > This means that sizeof(buf) is always 8 bytes, and the condition will
> > almost always be true, so we will reallocate the buffer more often
> > than we need to.
> >
> > Correct the condition to test against tx_buffer_size, which is
> > where we track how big the allocated buffer is.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/net/npcm_gmac.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>