[lwip-users] [lwip] MEM_ALIGN and pbuf.c

[Recker, John]

It appears to me that there are several problems 
with the use of MEM_ALIGN in pbuf.c (the code is
from the 5.3 release, but appears unchanged in the
latest release)

Two that I noticed are:

pbuf_alloc(). Line 235..

    /* Set the payload pointer so that it points offset bytes into
       pbuf data memory. */
    p->payload = MEM_ALIGN((void *)((u8_t *)p + (sizeof(struct pbuf) + 
offset)));

    /* The total length of the pbuf is the requested size. */
    p->tot_len = size;

    /* Set the length of the first pbuf is the chain. */
    p->len = size > PBUF_POOL_BUFSIZE - offset? PBUF_POOL_BUFSIZE - offset: 
size;

The problem here is that p->len is not correct 
if (p->payload != p). The size calculation should 
also account for incrementing p->payload
for alignment.


pbuf_alloc(). Line 262

      q->payload = (void *)((u8_t *)q + sizeof(struct pbuf));

This should be wrapped with the MEM_ALIGN macro.

I think that there are additional problems. 
But, at this point I started looking at the 
base cause of the memory alignment problem.
pbuf_pool_memory itself is guaranteed to be 
memory aligned. So, as long as PBUF_POOL_BUFSIZE
and sizeof(struct pbuf) are multiples of the 
memory alignment, then so should any address 
returned by pbuf_pool_alloc. 

So I was able to work around the problem by 
changing the definition of struct pbuf (note 
that I am forcing byte alignment in all my 
structs due to requirements of another library 
I am using). True, this change adds 2 bytes/struct, 
but the change does provide up to 64 bit alignment 
if it is packed (and the code won't work on
machines w/ memory alignment requirements w/o
the extra 2 bytes anyways...)

struct pbuf {
  struct pbuf *next;
  
  void *payload;

  /* high 4 bits, flags, low 4 bits reference count */
  u16_t flags, ref;
  
  /* Total length of buffer + additionally chained buffers. */
  u16_t tot_len;
  /* Length of this buffer. */
  u16_t len;  
  
};


jr



[This message was sent through the lwip discussion list.]