Re: [lwip-devel] Odd check in pbuf_alloced_custom (typo?)

[Mason]

Mason wrote:

> Hello,
> 
> My calls to pbuf_alloced_custom() are failing, and I don't
> understand the logic for the failure.
> 
> pbuf_alloced_custom is documented the following way:
> 
> /** Initialize a custom pbuf (already allocated).
>  *
>  * @param layer flag to define header size
>  * @param length size of the pbuf's payload
>  * @param type type of the pbuf (only used to treat the pbuf accordingly, as
>  *        this function allocates no memory)
>  * @param p pointer to the custom pbuf to initialize (already allocated)
>  * @param payload_mem pointer to the buffer that is used for payload and 
> headers,
>  *        must be at least big enough to hold 'length' plus the header size,
>  *        may be NULL if set later
>  * @param payload_mem_len the size of the 'payload_mem' buffer, must be at 
> least
>  *        big enough to hold 'length' plus the header size
>  */
> struct pbuf *pbuf_alloced_custom(
>   pbuf_layer l,
>   u16_t length,
>   pbuf_type type,
>   struct pbuf_custom *p,
>   void *payload_mem,
>   u16_t payload_mem_len)
> 
> If I understand correctly, 'payload_mem' points to a buffer which can
> store 'payload_mem_len' octets; length is the ACTUAL size of the
> payload.
> 
> Thus (please correct me if I am mistaken) supposing I have allocated
> 1600 octets for a buffer, and I have received an Ethernet frame that
> is 120-octets long, then
> 
>   length = 120 and payload_mem_len = 1600
> 
> correct?
> 
> However, pbuf_alloced_custom performs the following check:
> 
> if (LWIP_MEM_ALIGN_SIZE(offset) + length < payload_mem_len)
> {
>   /*** ERROR ***/
> }
> 
> which seems to be the opposite of what the comments (and logic)
> suggest.
> 
> If payload_mem_len "must be at least big enough to hold 'length'
> plus the header size", then there is an error when
> 
> LWIP_MEM_ALIGN_SIZE(offset) + length > payload_mem_len
> 
> Is this just a typo, with the comparison operator reversed?
> Or am I completely misunderstanding the logic of the code?

To be more precise, the (trivial) patch I am proposing is
the following.

--- pbuf.c.orig 2011-05-06 10:51:25.000000000 +0200
+++ pbuf.c      2011-08-24 11:02:34.484375000 +0200
@@ -369,7 +369,7 @@
     return NULL;
   }

-  if (LWIP_MEM_ALIGN_SIZE(offset) + length < payload_mem_len) {
+  if (LWIP_MEM_ALIGN_SIZE(offset) + length > payload_mem_len) {
     LWIP_DEBUGF(PBUF_DEBUG | LWIP_DBG_LEVEL_WARNING, 
("pbuf_alloced_custom(length=%"U16_F") buffer too short\n", length));
     return NULL;
   }

-- 
Regards.