diff --git a/src/core/dhcp.c b/src/core/dhcp.c index 5292fa79..4a39cf08 100644 --- a/src/core/dhcp.c +++ b/src/core/dhcp.c @@ -1105,7 +1105,7 @@ static void dhcp_free_reply(struct dhcp *dhcp) /** - * If an incoming DHCP message is in response to use, then trigger the state machine + * If an incoming DHCP message is in response to us, then trigger the state machine */ static void dhcp_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, struct ip_addr *addr, u16_t port) { @@ -1116,15 +1116,14 @@ static void dhcp_recv(void *arg, struct udp_pcb *pcb, struct pbuf *p, struct ip_ u8_t msg_type; u8_t i; DEBUGF(DHCP_DEBUG | DBG_TRACE | 3, ("dhcp_recv(pbuf = %p) from DHCP server %u.%u.%u.%u port %u\n", p, - (u8_t)(ntohl(addr->addr) >> 24 & 0xff), - (u8_t)(ntohl(addr->addr) >> 16 & 0xff), - (u8_t)(ntohl(addr->addr) >> 8 & 0xff), - (u8_t)(ntohl(addr->addr) & 0xff), port)); + (u8_t)(ntohl(addr->addr) >> 24 & 0xff), (u8_t)(ntohl(addr->addr) >> 16 & 0xff), + (u8_t)(ntohl(addr->addr) >> 8 & 0xff), (u8_t)(ntohl(addr->addr) & 0xff), port)); DEBUGF(DHCP_DEBUG | DBG_TRACE, ("pbuf->len = %u\n", p->len)); DEBUGF(DHCP_DEBUG | DBG_TRACE, ("pbuf->tot_len = %u\n", p->tot_len)); - /* prevent warning */ - if (pcb || addr || port); + /* prevent warnings about unused arguments */ + (void)pcb; (void)addr; (void)port; dhcp->p = p; + /* TODO: check packet length before reading them */ if (reply_msg->op != DHCP_BOOTREPLY) { DEBUGF(DHCP_DEBUG | DBG_TRACE | 1, ("not a DHCP reply message, but type %u\n", reply_msg->op)); pbuf_free(p); diff --git a/src/core/ipv6/icmp6.c b/src/core/ipv6/icmp6.c index 9237b5ca..4a899029 100644 --- a/src/core/ipv6/icmp6.c +++ b/src/core/ipv6/icmp6.c @@ -50,12 +50,13 @@ icmp_input(struct pbuf *p, struct netif *inp) struct icmp_echo_hdr *iecho; struct ip_hdr *iphdr; struct ip_addr tmpaddr; - #ifdef ICMP_STATS ++lwip_stats.icmp.recv; #endif /* ICMP_STATS */ + /* TODO: check length before accessing payload! */ + type = ((char *)p->payload)[0]; switch (type) { @@ -103,8 +104,7 @@ icmp_input(struct pbuf *p, struct netif *inp) iphdr->hoplim, IP_PROTO_ICMP, inp); break; default: - DEBUGF(ICMP_DEBUG, ("icmp_input: ICMP type not supported.\n")); - + DEBUGF(ICMP_DEBUG, ("icmp_input: ICMP type %d not supported.\n", (int)type)); #ifdef ICMP_STATS ++lwip_stats.icmp.proterr; ++lwip_stats.icmp.drop; diff --git a/src/core/pbuf.c b/src/core/pbuf.c index c4932c26..b42d2c9b 100644 --- a/src/core/pbuf.c +++ b/src/core/pbuf.c @@ -10,13 +10,14 @@ * A packet may span over multiple pbufs, chained as a singly linked * list. This is called a "pbuf chain". * - * Multiple packets may be queued using this singly linked list. This - * is called a "pbuf queue". So, a pbuf queue consists of one or more - * pbuf chains, each of which consist of one or more pbufs. + * Multiple packets may be queued, also using this singly linked list. + * This is called a "packet queue". So, a packet queue consists of one + * or more pbuf chains, each of which consist of one or more pbufs. * - * In order to find the last pbuf of a packet, traverse the linked list - * until the ->tot_len field equals the ->len field. If the ->next field - * of this packet is not NULL, more packets are on the queue. + * The last pbuf of a packet has a ->tot_len field that equals the + * ->len field. It can be found by traversing the list. If the last + * pbuf of a packet has a ->next field other than NULL, more packets + * are on the queue. */ /* @@ -366,6 +367,7 @@ pbuf_alloc(pbuf_layer l, u16_t length, pbuf_flag flag) * resized, and any remaining pbufs will be freed. * * @note If the pbuf is ROM/REF, only the ->tot_len and ->len fields are adjusted. + * @note May not be called on a packet queue. * * @bug Cannot grow the size of a pbuf (chain) (yet). */ @@ -442,6 +444,8 @@ pbuf_realloc(struct pbuf *p, u16_t new_len) * the call will fail. A check is made that the increase in header size does * not move the payload pointer in front of the start of the buffer. * @return 1 on failure, 0 on success. + * + * @note May not be called on a packet queue. */ u8_t pbuf_header(struct pbuf *p, s16_t header_size) @@ -500,6 +504,7 @@ pbuf_header(struct pbuf *p, s16_t header_size) * @return the number of unreferenced pbufs that were de-allocated * from the head of the chain. * + * @note May not be called on a packet queue. * @note the reference counter of a pbuf equals the number of pointers * that refer to the pbuf (or into the pbuf). * @@ -593,6 +598,7 @@ pbuf_clen(struct pbuf *p) } return len; } + /** * * Increment the reference count of the pbuf. @@ -618,6 +624,7 @@ pbuf_ref(struct pbuf *p) * * @param h head pbuf (chain) * @param t tail pbuf (chain) + * @note May not be called on a packet queue. * * The ->tot_len fields of all pbufs of the head chain are adjusted. * The ->next field of the last pbuf of the head chain is adjusted. @@ -651,8 +658,9 @@ pbuf_chain(struct pbuf *h, struct pbuf *t) DEBUGF(PBUF_DEBUG | DBG_FRESH | 2, ("pbuf_chain: referencing tail %p\n", (void *) t)); } -/* TODO: Will be enabled soon. Please review code. */ -#if 1 +/* For packet queueing. Note that queued packets must be dequeued first + * before calling any pbuf functions. */ +#if ARP_QUEUEING /** * Add a packet to the end of a queue. * @@ -680,6 +688,8 @@ pbuf_queue(struct pbuf *p, struct pbuf *n) p = p->next; } #endif + /* now p->tot_len == p->len */ + /* proceed to next packet on queue */ p = p->next; } /* chain last pbuf of h chain (p) with first of tail (t) */ @@ -714,7 +724,7 @@ pbuf_dequeue(struct pbuf *p) p->next = NULL; /* q is now referenced to one less time */ pbuf_free(q); - DEBUGF(PBUF_DEBUG | DBG_FRESH | 2, ("pbuf_dequeue: dereferencing remaining queue%p\n", (void *)q)); + DEBUGF(PBUF_DEBUG | DBG_FRESH | 2, ("pbuf_dequeue: dereferencing remaining queue %p\n", (void *)q)); return q; } #endif @@ -831,6 +841,7 @@ pbuf_take(struct pbuf *p) * Makes p->tot_len field equal to p->len. * @param p pbuf to dechain * @return remainder of the pbuf chain, or NULL if it was de-allocated. + * @note May not be called on a packet queue. */ struct pbuf * pbuf_dechain(struct pbuf *p) diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index af6dd595..0f61f08d 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -112,8 +112,9 @@ tcp_input(struct pbuf *p, struct netif *inp) iphdr = p->payload; tcphdr = (struct tcp_hdr *)((u8_t *)p->payload + IPH_HL(iphdr) * 4); - - if (pbuf_header(p, -((s16_t)(IPH_HL(iphdr) * 4)))) { + + /* remove header from payload */ + if (pbuf_header(p, -((s16_t)(IPH_HL(iphdr) * 4))) || (p->tot_len < sizeof(struct tcp_hdr))) { /* drop short packets */ DEBUGF(TCP_INPUT_DEBUG, ("tcp_input: short packet (%u bytes) discarded\n", p->tot_len)); #ifdef TCP_STATS diff --git a/src/include/lwip/mem.h b/src/include/lwip/mem.h index 17ff1ae6..f1ca270d 100644 --- a/src/include/lwip/mem.h +++ b/src/include/lwip/mem.h @@ -49,9 +49,9 @@ void mem_free(void *mem); void *mem_realloc(void *mem, mem_size_t size); void *mem_reallocm(void *mem, mem_size_t size); -#define MEM_ALIGN_SIZE(size) ((size + MEM_ALIGNMENT - 1) & ~(MEM_ALIGNMENT-1)) +#define MEM_ALIGN_SIZE(size) (((size) + MEM_ALIGNMENT - 1) & ~(MEM_ALIGNMENT-1)) -#define MEM_ALIGN(addr) (void *)MEM_ALIGN_SIZE((mem_ptr_t)addr) +#define MEM_ALIGN(addr) ((void *)(((mem_ptr_t)(addr) + MEM_ALIGNMENT - 1) & ~(mem_ptr_t)(MEM_ALIGNMENT-1))) #endif /* __LWIP_MEM_H__ */ diff --git a/src/include/lwip/pbuf.h b/src/include/lwip/pbuf.h index 01d0e9b1..26288302 100644 --- a/src/include/lwip/pbuf.h +++ b/src/include/lwip/pbuf.h @@ -72,7 +72,10 @@ struct pbuf { /** * total length of this buffer and all next buffers in chain - * invariant: p->tot_len == p->len + (p->next? p->next->tot_len: 0) + * belonging to the same packet. + * + * For non-queue packet chains this is the invariant: + * p->tot_len == p->len + (p->next? p->next->tot_len: 0) */ u16_t tot_len;