Re: [lwip-users] questions on lwip (two TCP/IP sockets)

[Jeff Barber]

I'd like to see someone more authoritative (like Simon or Kieran?)
weigh in on this but I'm pretty sure that the advice you cited is just
plain wrong.

It's true that data may be delivered to your receive callback in a
chain of pbufs (something your code is not currently handling); I
believe this can happen due to out-of-order tcp segments among other
things.  However, pbuf_free definitely chases down that chain to free
the linked buffers (read the source: lwip/src/core/pbuf.c).

A couple of other notes:
The pbuf structure is reference-counted so there are some cases when
calling pbuf_free will not (indeed must not) actually free it.  The
memory should only be released when the reference count goes to zero.
However, simply calling pbuf_free once here does the right thing.

Also, pbuf_free does not return an error code.  It returns the number
of pbuf structures that were deallocated.  It is difficult to see how
this would be useful to the caller except perhaps in some special
circumstance.

So my recommendation (as to freeing the memory) is that this is
completely sufficient for your receive callback:
  pbuf_free(p);

Jeff


On Thu, Aug 12, 2010 at 2:19 PM, shogun <address@hidden> wrote:
> Thanks for the help it is working much better now!  The comments about
> freeing memory is from this thread:
>
> http://e2e.ti.com/support/microcontrollers/stellaris_arm_cortex-m3_microcont
> roller/f/471/p/54250/192785.aspx#192785
>
> How do you recommend I free the memory?
>
> ----------------------------------------------------------------------
>
> <SNIP>
>
>
>> static err_t socket_new_recv(void *arg, struct tcp_pcb *pcb, struct pbuf
> *p,
>> err_t err )
>>
>> {
> [snip]
>>                                 ert = pbuf_free(p);
>>
>>                                 while(p->next) //see if next contains a
> non
>> NULL pointer
>>                                 {
>>                                                 ert = pbuf_free(p->next);
>>                                                 if(ert)
>>                                                 {
>>
> UARTprintf("
>> error (%d) freeing data at %x\n", ert, p->next);
>>                                                 }
>>                                                 p = p->next;
>>                                 }//end clean up memory
>>
> [snip]
>>                                 ert = pbuf_free(p);
>
> You should only be calling pbuf_free once, and if the pbuf is chained,
> pbuf_free will chase the chain for you, you do not need to do so.
> (And if you did need to chase the chain, it's generally not a good
> idea to be looking at a data structure's contents after you've freed
> it, as you're doing here.)
>
> Jeff
>
>
>
>
>
> _______________________________________________
> lwip-users mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/lwip-users
>