Re: [lwip-devel] pbuf_chain followed by pbuf_free

[Leon Woestenberg]

Hello all,
>
> I see that almost all (7 of ~10) uses of the form pbuf_chain(p,q) are
> followed by pbuf_free(q). It seems to me that having a new function i.e
>
Hmm, I would prefer not changing the API and be suboptimal. Although,
did you look into which of the pbuf_chain()s are most often used, those
with or without _free()?

I think we can only optimize lwIP after a decent amount of profiling.

Anyway, it looks like your proposed change can be implemented
non-intrusively in a step-by-step approach:

1 - implement pbuf_cat() as a copy of pbuf_chain(), without increment tail's
reference count.
2 - make pbuf_chain() do  pbuf_ref(tail); pbuf_cat(head,tail);
3 - change the pbuf_chain()/free() combinations (which now have three
locks!) into pbuf_cat()
in the lwIP stack as well as the contributions.

I am strongly against having both a pbuf_chain() and pbuf_cat() full
implementation, for
code size and maintenance reasons.

Also, I do not think the names are more clear in your proposal. They do not
indicate that
the caller of pbuf_cat() may no longer reference the tail pbuf.

This is much clearer in the current situation, where the caller explicitly
calls pbuf_free(tail)
and where the programmer is enforced not to reference tail afterwards.

Especially the latter argument would want me to say: keep it as is.

Both ways are fine with me. We do not use the pbuf locks anyway, we use a
coarser
locking mechanism as I do not trust the correctness of the lwIP locking
schemes in
a pre-emptive real-time environment with nested interrupts etc.

Regards,

Leon Woestenberg.