[lwip-devel] [bug #11400] ARP multi-packet-queue modifies TCP unsent/unacked segment packet pbuf chain into packet queue

[Jonathan Larmour]

Follow-up Comment #40, bug #11400 (project lwip):

>From visual inspection:
pbuf_copy returns q. It should return r. I think q could be renamed to
something like p_copy, and r to p_copy_head to make their purposes clearer.

An assert should trigger at the point it says:
       /* this should not happen! */
I don't think we should control programming errors - that's what asserts are
for, and if we're too defensive, that adds code bloat to lwip.
So it should instead it should just be:
LWIP_ASSERT( "pbuf_copy: Reached unexpected end of copy", q != NULL );

Next, there's an assumption that if the current chunk of p doesn't fit into
the current chunk of q, then after we copy part of p to the current q and
move q to the next q, the rest of p will fit into that next q. Instead there
should probably be something like a "continue" to restart the loop (and the
offset into p preserved) at the end of the "if(p->len > rem_len)" block.

As per comment #34 and comment #35, I thought that after all we would also
need to copy RAM pbufs because the user may otherwise be allowed to change
data in the pbuf once control is returned to them (this is a weakness of the
API).

A final comment - I think a PBUF_RAM should be attempted before resorting to
PBUF_POOL. PBUF_POOL packets may be much much larger than the data for this
packet needs (and the size of PBUF_POOL packets is normally chosen by the
user according to the requirements of receive data, not outgoing data).

Thanks,

Jifl


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?11400>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/