Added assertions where PBUF_RAM pbufs are used and an assumption is made that this pbuf is in one piece (i.e. not chained). These assumptions clash with the possibility of converting to fully pool-based pbuf implementations, where PBUF_RAM pbufs might be chained.

CHANGELOG

@@ -241,6 +241,12 @@ HISTORY
 
   ++ Bug fixes:
 
+  2007-07-03 Simon Goldschmidt
+  * nearly-all-files: Added assertions where PBUF_RAM pbufs are used and an
+    assumption is made that this pbuf is in one piece (i.e. not chained). These
+    assumptions clash with the possibility of converting to fully pool-based
+    pbuf implementations, where PBUF_RAM pbufs might be chained.
+
   2007-07-03 Simon Goldschmidt
   * api.h, api_lib.c, api_msg.c: Final fix for bug #20021 and some other problems
     when closing tcp netconns: removed conn->sem, less context switches when

src/api/api_lib.c

@@ -108,6 +108,8 @@ netbuf_alloc(struct netbuf *buf, u16_t size)
   if (buf->p == NULL) {
      return NULL;
   }
+  LWIP_ASSERT("check that first pbuf can hold size",
+             (buf->p->len >= size));
   buf->ptr = buf->p;
   return buf->p->payload;
 }

src/core/dhcp.c

@@ -1312,6 +1312,9 @@ static err_t dhcp_create_request(struct netif *netif)
     LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("dhcp_create_request(): could not allocate pbuf\n"));
     return ERR_MEM;
   }
+  LWIP_ASSERT("check that first pbuf can hold struct dhcp_msg",
+           (dhcp->p_out->len >= sizeof(struct dhcp_msg)));
+
   /* give unique transaction identifier to this request */
   dhcp->xid = xid++;
   LWIP_DEBUGF(DHCP_DEBUG | LWIP_DBG_TRACE | 2, ("transaction id xid++(%"X32_F") dhcp->xid(%"U32_F")\n",xid,dhcp->xid));

src/core/ipv4/icmp.c

@@ -112,6 +112,8 @@ icmp_input(struct pbuf *p, struct netif *inp)
         LWIP_DEBUGF(ICMP_DEBUG, ("icmp_input: allocating new pbuf failed\n"));
         goto memerr;
       }
+      LWIP_ASSERT("check that first pbuf can hold struct the ICMP header",
+                  (r->len >= hlen + sizeof(struct icmp_echo_hdr)));
       /* copy the whole packet including ip header */
       if (pbuf_copy(r, p) != ERR_OK) {
         LWIP_ASSERT("icmp_input: copying to new pbuf failed\n", 0);
@@ -201,8 +203,16 @@ icmp_dest_unreach(struct pbuf *p, enum icmp_dur_type t)
   struct ip_hdr *iphdr;
   struct icmp_dur_hdr *idur;
 
+  /* @todo: can this be PBUF_LINK instead of PBUF_IP? */
   q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM);
   /* ICMP header + IP header + 8 bytes of data */
+  if (q == NULL) {
+    LWIP_DEBUGF(ICMP_DEBUG, ("icmp_dest_unreach: failed to allocate pbuf for ICMP packet.\n"));
+    pbuf_free(p);
+    return;
+  }
+  LWIP_ASSERT("check that first pbuf can hold icmp message",
+             (q->len >= (8 + IP_HLEN + 8)));
 
   iphdr = p->payload;
 
@@ -240,7 +250,16 @@ icmp_time_exceeded(struct pbuf *p, enum icmp_te_type t)
   struct ip_hdr *iphdr;
   struct icmp_te_hdr *tehdr;
 
+  /* @todo: can this be PBUF_LINK instead of PBUF_IP? */
   q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM);
+  /* ICMP header + IP header + 8 bytes of data */
+  if (q == NULL) {
+    LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded: failed to allocate pbuf for ICMP packet.\n"));
+    pbuf_free(p);
+    return;
+  }
+  LWIP_ASSERT("check that first pbuf can hold icmp message",
+             (q->len >= (8 + IP_HLEN + 8)));
 
   iphdr = p->payload;
   LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded from "));

src/core/ipv4/igmp.c

@@ -562,6 +562,8 @@ igmp_send(struct igmp_group *group, u8_t type)
   
   if (p) {
     igmp = p->payload;
+    LWIP_ASSERT("check that first pbuf can hold struct igmpmsg",
+               (p->len >= sizeof(struct igmpmsg)));
     ip_addr_set(&src, &((group->interface)->ip_addr));
      
     if (IGMP_V2_MEMB_REPORT == type) {

src/core/ipv4/ip.c

@@ -460,6 +460,8 @@ ip_output_if(struct pbuf *p, struct ip_addr *src, struct ip_addr *dest,
     }
 
     iphdr = p->payload;
+    LWIP_ASSERT("check that first pbuf can hold struct ip_hdr",
+               (p->len >= sizeof(struct ip_hdr)));
 
     IPH_TTL_SET(iphdr, ttl);
     IPH_PROTO_SET(iphdr, proto);

src/core/ipv4/ip_frag.c

@@ -403,6 +403,8 @@ ip_frag(struct pbuf *p, struct netif *netif, struct ip_addr *dest)
     if (rambuf == NULL) {
       return ERR_MEM;
     }
+    LWIP_ASSERT("this needs a pbuf in one piece!",
+                (p->len >= (IP_HLEN)));
     SMEMCPY(rambuf->payload, original_iphdr, IP_HLEN);
     iphdr = rambuf->payload;
 

src/core/ipv6/icmp6.c

@@ -121,8 +121,16 @@ icmp_dest_unreach(struct pbuf *p, enum icmp_dur_type t)
   struct ip_hdr *iphdr;
   struct icmp_dur_hdr *idur;
 
+  /* @todo: can this be PBUF_LINK instead of PBUF_IP? */
   q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM);
   /* ICMP header + IP header + 8 bytes of data */
+  if (q == NULL) {
+    LWIP_DEBUGF(ICMP_DEBUG, ("icmp_dest_unreach: failed to allocate pbuf for ICMP packet.\n"));
+    pbuf_free(p);
+    return;
+  }
+  LWIP_ASSERT("check that first pbuf can hold icmp message",
+             (q->len >= (8 + IP_HLEN + 8)));
 
   iphdr = p->payload;
 
@@ -153,7 +161,16 @@ icmp_time_exceeded(struct pbuf *p, enum icmp_te_type t)
 
   LWIP_DEBUGF(ICMP_DEBUG, ("icmp_time_exceeded\n"));
 
+  /* @todo: can this be PBUF_LINK instead of PBUF_IP? */
   q = pbuf_alloc(PBUF_IP, 8 + IP_HLEN + 8, PBUF_RAM);
+  /* ICMP header + IP header + 8 bytes of data */
+  if (q == NULL) {
+    LWIP_DEBUGF(ICMP_DEBUG, ("icmp_dest_unreach: failed to allocate pbuf for ICMP packet.\n"));
+    pbuf_free(p);
+    return;
+  }
+  LWIP_ASSERT("check that first pbuf can hold icmp message",
+             (q->len >= (8 + IP_HLEN + 8)));
 
   iphdr = p->payload;
 

src/core/pbuf.c

@@ -153,7 +153,7 @@ pbuf_alloc(pbuf_layer l, u16_t length, pbuf_flag flag)
   switch (flag) {
   case PBUF_POOL:
     /* allocate head of pbuf chain into p */
-    p = memp_malloc(MEMP_PBUF_POOL);
+      p = memp_malloc(MEMP_PBUF_POOL);
     LWIP_DEBUGF(PBUF_DEBUG | LWIP_DBG_TRACE | 3, ("pbuf_alloc: allocated pbuf %p\n", (void *)p));
     if (p == NULL) {
       return NULL;
@@ -184,7 +184,7 @@ pbuf_alloc(pbuf_layer l, u16_t length, pbuf_flag flag)
     rem_len = length - p->len;
     /* any remaining pbufs to be allocated? */
     while (rem_len > 0) {
-      q = memp_malloc(MEMP_PBUF_POOL);
+        q = memp_malloc(MEMP_PBUF_POOL);
       if (q == NULL) {
         /* free chain so far allocated */
         pbuf_free(p);

src/core/tcp_out.c

@@ -214,6 +214,8 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
       if ((seg->p = pbuf_alloc(PBUF_TRANSPORT, optlen, PBUF_RAM)) == NULL) {
         goto memerr;
       }
+      LWIP_ASSERT("check that first pbuf can hold optlen",
+                  (seg->p->len >= optlen));
       ++queuelen;
       seg->dataptr = seg->p->payload;
     }
@@ -223,6 +225,8 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
         LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 2, ("tcp_enqueue : could not allocate memory for pbuf copy size %"U16_F"\n", seglen));
         goto memerr;
       }
+      LWIP_ASSERT("check that first pbuf can hold the complete seglen",
+                  (seg->p->len >= seglen));
       ++queuelen;
       if (arg != NULL) {
         MEMCPY(seg->p->payload, ptr, seglen);
@@ -664,6 +668,8 @@ tcp_rst(u32_t seqno, u32_t ackno,
       LWIP_DEBUGF(TCP_DEBUG, ("tcp_rst: could not allocate memory for pbuf\n"));
       return;
   }
+  LWIP_ASSERT("check that first pbuf can hold struct tcp_hdr",
+              (p->len >= sizeof(struct tcp_hdr)));
 
   tcphdr = p->payload;
   tcphdr->src = htons(local_port);
@@ -784,6 +790,8 @@ tcp_keepalive(struct tcp_pcb *pcb)
       LWIP_DEBUGF(TCP_DEBUG, ("tcp_keepalive: could not allocate memory for pbuf\n"));
       return;
    }
+  LWIP_ASSERT("check that first pbuf can hold struct tcp_hdr",
+              (p->len >= sizeof(struct tcp_hdr)));
 
    tcphdr = p->payload;
    tcphdr->src = htons(pcb->local_port);

src/core/udp.c

@@ -379,6 +379,8 @@ udp_send(struct udp_pcb *pcb, struct pbuf *p)
     q = p;
     LWIP_DEBUGF(UDP_DEBUG, ("udp_send: added header in given pbuf %p\n", (void *)p));
   }
+  LWIP_ASSERT("check that first pbuf can hold struct udp_hdr",
+              (q->len >= sizeof(struct udp_hdr)));
   /* q now represents the packet to be sent */
   udphdr = q->payload;
   udphdr->src = htons(pcb->local_port);

src/netif/etharp.c

@@ -1028,6 +1028,8 @@ etharp_raw(struct netif *netif, const struct eth_addr *ethsrc_addr,
     LWIP_DEBUGF(ETHARP_DEBUG | LWIP_DBG_TRACE | 2, ("etharp_raw: could not allocate pbuf for ARP request.\n"));
     return ERR_MEM;
   }
+    LWIP_ASSERT("check that first pbuf can hold struct etharp_hdr",
+                (p->len >= sizeof(struct etharp_hdr)));
 
   hdr = p->payload;
   LWIP_DEBUGF(ETHARP_DEBUG | LWIP_DBG_TRACE, ("etharp_raw: sending raw ARP packet.\n"));

src/netif/loopif.c

@@ -61,7 +61,7 @@ void
 loopif_poll(struct netif *netif)
 {
   SYS_ARCH_DECL_PROTECT(lev);
-  struct pbuf *in = NULL;
+  struct pbuf *in, *in_end;
   struct loopif_private *priv = (struct loopif_private*)netif->state;
 
   LWIP_ERROR("priv != NULL", (priv != NULL), return;);
@@ -70,30 +70,35 @@ loopif_poll(struct netif *netif)
     /* Get a packet from the list. With SYS_LIGHTWEIGHT_PROT=1, this is protected */
     SYS_ARCH_PROTECT(lev);
     in = priv->first;
-    if(priv->first) {
-      if(priv->first == priv->last) {
+    if(in) {
+      in_end = in;
+      while(in_end->len != in_end->tot_len) {
+        LWIP_ASSERT("bogus pbuf: len != tot_len but next == NULL!", in_end->next != NULL);
+        in_end = in_end->next;
+      }
+      /* 'in_end' now points to the last pbuf from 'in' */
+      if(in_end == priv->last) {
         /* this was the last pbuf in the list */
         priv->first = priv->last = NULL;
       } else {
         /* pop the pbuf off the list */
-        priv->first = priv->first->next;
+        priv->first = in_end->next;
         LWIP_ASSERT("should not be null since first != last!", priv->first != NULL);
       }
     }
     SYS_ARCH_UNPROTECT(lev);
   
     if(in != NULL) {
-      if(in->next != NULL) {
+      if(in_end->next != NULL) {
         /* De-queue the pbuf from its successors on the 'priv' list. */
-        in->next = NULL;
-        /* This is built on the assumption that PBUF_RAM pbufs are in one piece! */
-        LWIP_ASSERT("packet must not consist of multiple pbufs!", in->len == in->tot_len);
+        in_end->next = NULL;
       }
       if(netif->input(in, netif) != ERR_OK) {
         pbuf_free(in);
       }
       /* Don't reference the packet any more! */
       in = NULL;
+      in_end = NULL;
     }
   /* go on while there is a packet on the list */
   } while(priv->first != NULL);
@@ -121,6 +126,7 @@ loopif_output(struct netif *netif, struct pbuf *p,
 #if !LWIP_LOOPIF_MULTITHREADING
   SYS_ARCH_DECL_PROTECT(lev);
   struct loopif_private *priv;
+  struct pbuf *last;
 #endif /* LWIP_LOOPIF_MULTITHREADING */
   struct pbuf *r;
   err_t err;
@@ -153,16 +159,16 @@ loopif_output(struct netif *netif, struct pbuf *p,
      through calling loopif_poll(). */
   priv = (struct loopif_private*)netif->state;
 
-  /* This is built on the assumption that PBUF_RAM pbufs are in one piece! */
-  LWIP_ASSERT("packet must not consist of multiple pbufs!", r->len == r->tot_len);
-
+  /* let last point to the last pbuf in chain r */
+  for (last = r; last->next != NULL; last = last->next);
   SYS_ARCH_PROTECT(lev);
   if(priv->first != NULL) {
     LWIP_ASSERT("if first != NULL, last must also be != NULL", priv->last != NULL);
     priv->last->next = r;
-    priv->last = r;
+    priv->last = last;
   } else {
-    priv->first = priv->last = r;
+    priv->first = r;
+    priv->last = last;
   }
   SYS_ARCH_UNPROTECT(lev);
 #endif /* LWIP_LOOPIF_MULTITHREADING */