Re: [lwip-users] Bug in SLIPIF.C ?

[Winformatik Info]

Hi

Sorry, I forgot to mention that it is working now.

Here a short description what happens without the correction.

- If the slip driver the following code will be executed if a new buffer 
will be allocated:
     p = pbuf_alloc(PBUF_LINK, PBUF_POOL_BUFSIZE, PBUF_POOL);

  PBUF_POOL_BUFSIZE is set to 0x80

- In pbuf_alloc() the offset in the PBUF_LINK case will be calculated 
(BPUF_LINK_HLEN is set to 0x10)
     offset += PBUF_LINK_HLEN;
 
- In the case PBUF_POOL the first buffer will be allocated
     /* allocate head of pbuf chain into p */
     p = pbuf_pool_alloc();

- After that p->tot_len will be set and p->len will be calculated
     p->len = length > PBUF_POOL_BUFSIZE - offset? PBUF_POOL_BUFSIZE - 
offset: length;

  Calculated p->len in this case will be 0x70

- After that the rem_length (I guess means remaining length) will be 
calulated
     rem_len = length - p->len;

  This calculation wil be result in a value of rem_len of 0x10

- The following loop "while (rem_len > 0) {" will allocate buffer until 
rem_len is 0 or a negative value

I tested it with a little html download over the silp interface into the 
Opera browser. In this case the GET message from the browser is greater 
then the 128 bytes of a buffer. This will results in checksum errors, 
because the buffers where not filled with the correct data.
This means for the above description the buffer with the length 0x10 
would never be used, because after the first buffer is filled with data 
"p = NULL" will be executed and a new buffer will be allocated. The 
buffer with the length of 0x10 remains the buffer chain.

I hope this description is (more or less) usefull for you.

Regards, Roland


Jani Monoses wrote:

> Hello
>
> you did not say if this change made your slipif work or not.
>  
>
> > In the code fragment below there is the old faulty code and the new 
> > replacement:
> >
> >        if ((p != NULL) && (recved < MAX_SIZE)) {
> >          ((u8_t *)p->payload)[i] = c;
> >          recved++;
> >          i++;
> >          if (i >= p->len) {
> >            i = 0;
> > //            p = NULL;      FIXME problem if more then one buffer 
> > allocated by pbuf_alloc()
> >    
>
> When can that happen? the pbuf_alloc() in that code looks like it allocates
> one item in the pool (PBUF_POOL_BUFSIZE)
>  
>
> >            p = p->next;       // corrected code, to go to next buffer
> >          }
> >        }
> >
> > If no more data buffer space available in the chain, p->next holds a 
> > NULL value. This signals the driver to allocate new buffers.
> >
> > Is the correction above ok ?
> >    
>
> I forgot what I knew about this slipif code so I'll have to look closer to
> answer that :).
>
> Jani
>