[lwip-users] pbuf PBUF_POOL alignment [PATCH]

[Pedro Alves]

This patch fixes alignment problems of pbuf of PBUF_POOL type.

When the alignment forces the payload to move, the len was not 
accounting for it.

Example, for 4 byte alignment:
offset requested 12.
sizeof(struct pbuf) = 14
length = 200
PBUF_POOL_BUFSIZE 128

payload was rounded up by two bytes, like this:
p->payload = MEM_ALIGN((void *)((u8_t *)p + (sizeof(struct pbuf) + 
offset)));
p + 14 + 12 -> p + 14 + 12 + 2extra -> p + 28 (aligned)
later:
p->len = length > PBUF_POOL_BUFSIZE - offset? PBUF_POOL_BUFSIZE - 
offset: length;
p->len = 200 > 128 - 12 ? 128 - 12 : 200 -> 128-12 -> 116 !!!

Fixed it by doing:
offset = MEM_ALIGN_SIZE(offset);

Also, there were a few MEM_ALIGNs missing. Added them.

Cheers,
Pedro Alves


Index: CHANGELOG
===================================================================
RCS file: /sources/lwip/lwip/CHANGELOG,v
retrieving revision 1.41
diff -u -r1.41 CHANGELOG
--- CHANGELOG    7 Apr 2006 22:31:09 -0000    1.41
+++ CHANGELOG    13 Apr 2006 17:19:11 -0000
@@ -21,6 +21,10 @@

  * [Enter new changes just after this line - do not remove this line]

+  2006-04-13 Pedro Alves <address@hidden>
+  * pbuf.c: Fix alignment: pbuf with PBUF_POOL type was corrupting data 
when an
+    unaligned offset was requested. Always MEM_ALIGN the payload.
+
  2006-03-29 Christiaan Simons
  * inet.c, inet.h: Added platform byteswap support.
    Added LWIP_PLATFORM_BYTESWAP define (defaults to 0) and
Index: src/core/pbuf.c
===================================================================
RCS file: /sources/lwip/lwip/src/core/pbuf.c,v
retrieving revision 1.80
diff -u -r1.80 pbuf.c
--- src/core/pbuf.c    28 Mar 2006 15:06:33 -0000    1.80
+++ src/core/pbuf.c    13 Apr 2006 17:06:03 -0000
@@ -110,9 +110,9 @@
  p = pbuf_pool;

  for(i = 0; i < PBUF_POOL_SIZE; ++i) {
-    p->next = (struct pbuf *)((u8_t *)p + PBUF_POOL_BUFSIZE + 
sizeof(struct pbuf));
+    p->next = (struct pbuf *)((u8_t *)MEM_ALIGN((u8_t *)p + 
sizeof(struct pbuf)) + MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE));
    p->len = p->tot_len = PBUF_POOL_BUFSIZE;
-    p->payload = MEM_ALIGN((void *)((u8_t *)p + sizeof(struct pbuf)));
+    p->payload = MEM_ALIGN((u8_t *)p + sizeof(struct pbuf));
    p->flags = PBUF_FLAG_POOL;
    q = p;
    p = p->next;
@@ -252,9 +252,11 @@
    p->next = NULL;

    /* make the payload pointer point 'offset' bytes into pbuf data 
memory */
-    p->payload = MEM_ALIGN((void *)((u8_t *)p + (sizeof(struct pbuf) + 
offset)));
+    p->payload = MEM_ALIGN((u8_t *)p + (sizeof(struct pbuf) + offset));
    LWIP_ASSERT("pbuf_alloc: pbuf p->payload properly aligned",
            ((mem_ptr_t)p->payload % MEM_ALIGNMENT) == 0);
+    /* account for the possible alignment above shrinking the available 
space */
+    offset = MEM_ALIGN_SIZE(offset);
    /* the total length of the pbuf chain is the requested size */
    p->tot_len = length;
    /* set the length of the first pbuf in the chain */
@@ -288,7 +290,7 @@
      q->tot_len = rem_len;
      /* this pbuf length is pool size, unless smaller sized tail */
      q->len = rem_len > PBUF_POOL_BUFSIZE? PBUF_POOL_BUFSIZE: rem_len;
-      q->payload = (void *)((u8_t *)q + sizeof(struct pbuf));
+      q->payload =  MEM_ALIGN((void *)((u8_t *)q + sizeof(struct pbuf)));
      LWIP_ASSERT("pbuf_alloc: pbuf q->payload properly aligned",
              ((mem_ptr_t)q->payload % MEM_ALIGNMENT) == 0);
      q->ref = 1;
@@ -305,10 +307,12 @@
    /* If pbuf is to be allocated in RAM, allocate memory for it. */
    p = mem_malloc(MEM_ALIGN_SIZE(sizeof(struct pbuf) + offset) + 
MEM_ALIGN_SIZE(length));
    if (p == NULL) {
+      LWIP_DEBUGF(PBUF_DEBUG | DBG_TRACE | 2, ("pbuf_alloc: Could not 
allocate MEMP_PBUF for PBUF_RAM.\n"));
      return NULL;
    }
+
    /* Set up internal structure of the pbuf. */
-    p->payload = MEM_ALIGN((void *)((u8_t *)p + sizeof(struct pbuf) + 
offset));
+    p->payload = MEM_ALIGN((u8_t *)p + sizeof(struct pbuf) + offset);
    p->len = p->tot_len = length;
    p->next = NULL;
    p->flags = PBUF_FLAG_RAM;
@@ -586,7 +590,7 @@
      /* is this a pbuf from the pool? */
      if (flags == PBUF_FLAG_POOL) {
        p->len = p->tot_len = PBUF_POOL_BUFSIZE;
-        p->payload = (void *)((u8_t *)p + sizeof(struct pbuf));
+        p->payload =  MEM_ALIGN((u8_t *)p + sizeof(struct pbuf));
        PBUF_POOL_FREE(p);
      /* is this a ROM or RAM referencing pbuf? */
      } else if (flags == PBUF_FLAG_ROM || flags == PBUF_FLAG_REF) {