[lwip-devel] [bug #27034] Invalid ASSERT in pbuf_alloc()

[Iordan Neshev]

URL:
  <http://savannah.nongnu.org/bugs/?27034>

                 Summary: Invalid ASSERT in pbuf_alloc() 
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: iordan_neshev
            Submitted on: Wed 15 Jul 2009 12:45:31 PM GMT
                Category: pbufs
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: CVS Head

    _______________________________________________________

Details:

In pbuf.c:
struct pbuf *
pbuf_alloc(pbuf_layer layer, u16_t length, pbuf_type type)
{
...
  switch (type) {
  case PBUF_POOL:
...
 p->len = LWIP_MIN(length, PBUF_POOL_BUFSIZE_ALIGNED - 
                     LWIP_MEM_ALIGN_SIZE(offset));
...
LWIP_ASSERT("PBUF_POOL_BUFSIZE must be bigger than 
              MEM_ALIGNMENT", p->len != 0);
}

The logical assert is really
(PBUF_POOL_BUFSIZE_ALIGNED > LWIP_MEM_ALIGN_SIZE(offset)).

I guess it was replaced with (p->len != 0) with the 
intention to eliminate the overhead of evaluating the
LWIP_MEM_ALIGN_SIZE macro again. But when the requested
lenght is 0 this assert is wrong.

If I am right why this was done (to eliminate the overhead),
then the expression (PBUF_POOL_BUFSIZE_ALIGNED - LWIP_MEM_ALIGN_SIZE(offset))
could be explicitly 
evaluated only once by storing the result in a local variable. This adds a
few lines of source which I dislike. Alternatively
it may become:

LWIP_ASSERT("PBUF_POOL_BUFSIZE must be bigger than MEM_ALIGNMENT",
(PBUF_POOL_BUFSIZE_ALIGNED - LWIP_MEM_ALIGN_SIZE(offset)) > 0);
relying on the optimizer to evaluate it once.
I'm sure it will do it especially if 
this ASSERT is moved right before 
p->len = LWIP_MIN(length, ...

On the other hand it's (almost)worthless to optimize 
tests in assertions since in the field they are 
usually disabled.

I propose to replace
p->len != 0
with 
(PBUF_POOL_BUFSIZE_ALIGNED - LWIP_MEM_ALIGN_SIZE(offset)) > 0

Greetings,
Iordan





    _______________________________________________________

Reply to this item at:

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

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