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

[Leon Woestenberg]

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.