[Ucip-devel] Re: BUG found in nTrimQ in netbuf.c

[Robert D. <od...@pn...>]

Hi Mads,

Thanks for the fix. I have modified the code with your update 
and will drop it into CVS shortly. I left the various unused 

#ifdef XXX
            OS_ENTER_CRITICAL();
#endif

conditionals in place as I am unsure whether they are required 
in multitasking version and so will get back to that another 
time.

Regards, Robert.




At 01:21 PM 11/01/2002 +0100, Mads Christiansen wrote:
>Hi everyone!
>
>Please read this Robert.
>
>And some days of seeking i finally found and annoying bug.
>It seems that sometimes the transmit queue in the stacks gets corrupted.
>
>I located the error in the nTrimQ function in netbuf.c.
>Actually I'm not sure if any of you guys ever will see this error since
I'm running the stack in a single task.
>
>I would very much like you Robert to update the CVS, since my pc is not
configured for the CVS anymore.
>
>The error will sometimes occur when nTrim is called from nTrimQ. Read the
comments in the source below.
>
>
>Best wishes to you all,
>
>Mads Christiansen
>Partner Voxtream
>www.voxtream.com 
>
>
>Here is the new nTrimQ function.
>
>
>/*
> * nTrimQ - Trim bytes from the front of a buffer queue.
> * Note: The queue needs to be protected from collisions for the duration
> * of this call.
> * Return the number of bytes trimmed.
> */
>int nTrimQ(char *dst, NBufQHdr *qh, u_int len)
>{
>	int st = 0;
>	
>	if (qh && qh->qHead && len) {
>		NBuf *n0;
>		int trimmed;
>		
>		/* Trim entire chains. */
>		while (qh->qHead && len >= qh->qHead->chainLen) {
>
>			if ((qh->qHead = (n0 = qh->qHead)->nextChain) == NULL)
>				qh->qTail = NULL;
>			qh->qLen--;
>			
>			n0->nextChain = NULL;
>
>      if (dst)
>      {
>			  trimmed = nTrim(dst, &n0, len);
>			  if (dst)
>				  dst += trimmed;
>			  len -= trimmed;
>			  st += trimmed;
>      }
>      else
>      {
>        len -= n0->chainLen;
>        st += n0->chainLen;
>      }
>      if (n0) nFreeChain(n0);
>		}
>		
>		/* If more to go, trim from next chain. */
>		if (len && qh->qHead) {
>			/* 
>			 * XXX LONG CRITICAL SECTION!!!  Could we pop this off the queue,
>			 * trim it, and then replace the remainder?  Do we need a semaphore? 
>			 */
>      trimmed = nTrim(dst, &qh->qHead, len);
>			st += trimmed;
>
>      // Update tail pointer!!! SERIOUS BUGFIX /ma...@mo... 2002.01.11
>      if (qh->qLen == 1)
>      {
>        // nTrim might return a new nBuf in qh->qHead
>        // IF qLen == 1 we have to update qTrail!!! 
>        qh->qTail = qh->qHead;
>      }
>		}
>	}
>	
>	return st;
>}
>
>
>_______________________________________________
>Ucip-devel mailing list
>Uci...@li...
>https://lists.sourceforge.net/lists/listinfo/ucip-devel
>
>