Re: Re: [lwip-users] Found bug in pbuf.c

[Konrad, Guido]

Hello Leon,

I am not sure but please think about the following:

Two pbufs are allocated and chained together with pbuf_ref(t). After that,
the ref count of the first pbuf is 1, the count of the second is 2. For
three pbufs, you would see ref count 1, 2, 2.
Example: tcp_out.c in line 211, if you use netconn_write(... _NOCOPY). 

If you now try to free the chained pbuf, the code below decrements the ref
count of every chain member by one, means that only the first is freed. Do
you agree?
Example: tcp_seg_free(next); in tcp_in.c after ACK frees only the first
pbuf.

Please comment,

Regards,
-- Guido Konrad

-----Ursprüngliche Nachricht-----
Von: Leon Woestenberg [mailto:address@hidden
Gesendet: Mittwoch, 28. Mai 2003 01:13
An: Mailing list for lwIP users
Betreff: Re: [lwip-users] Found bug in pbuf.c


Hello Guido,

> in my last mail I asked for help due to a problem with memory leaks. It
> seems to me that this problem is related to the ref counter in pbufs if
> pbufs are chained. Function pbuf_chain increments the ref counter of the
> chained pbuf, (e.g. the second), but pbuf_free does not decrease the ref
> counter. So only the first pbuf of a chain is freed.
>
I have to disagree with that, but please see if I am correct.

Just after your added fix (see below), it reads "p = q;"

There, p is assigned to point to the next pbuf in chain (stored in q).

The while loop then... well... "loops" and p->ref-- is performed on
the second pbuf, which will be freed also if its ref count reaches zero.

So, I think your fix is wrong, and this function is OK.

The loop effectively does this:

  /* de-allocate all consecutive pbufs from the head of the chain that
   * obtain a zero reference count after decrementing once*/

>     while (p != NULL) {
>         /* all pbufs in a chain are referenced at least once */
>         LWIP_ASSERT("pbuf_free: p->ref > 0", p->ref > 0);
>         p->ref--;
>         /* this pbuf is no longer referenced to? */
>         if (p->ref == 0) {
>             /* remember next pbuf in chain for next iteration */
>             q = p->next;
>             /* is this a pbuf from the pool? */
>             if (p->flags == PBUF_FLAG_POOL) {
>                 p->len = p->tot_len = PBUF_POOL_BUFSIZE;
>                 p->payload = (void *)((u8_t *)p + sizeof(struct pbuf));
>                 PBUF_POOL_FREE(p);
>                 /* a RAM/ROM referencing pbuf */
>             } else if (p->flags == PBUF_FLAG_ROM || p->flags ==
> PBUF_FLAG_REF) {
>                 memp_freep(MEMP_PBUF, p);
>                 /* pbuf with data */
>             } else {
>                 mem_free(p);
>             }
>             count++;
>             /* proceed to next pbuf */
> gko->       if( q != NULL ) q->ref--;
>             p = q;
>             /* p->ref > 0, this pbuf is still referenced to */
>             /* (so the remaining pbufs in chain as well)    */
>         } else {
>             /* stop walking through chain */
>             p = NULL;
>         }
>     }
>
>
> Please comment.
>
I would suggest putting some extra debug outputs here if you can debug at
all.

I am very interested in knowing what is going on, as the stack should be
very
stable by now.

With regards,

Leon Woestenberg.




_______________________________________________
lwip-users mailing list
address@hidden
http://mail.nongnu.org/mailman/listinfo/lwip-users