ucip-devel

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

[Mads C. <MC...@vo...>]

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=20


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 =3D 0;
=09
	if (qh && qh->qHead && len) {
		NBuf *n0;
		int trimmed;
	=09
		/* Trim entire chains. */
		while (qh->qHead && len >=3D qh->qHead->chainLen) {

			if ((qh->qHead =3D (n0 =3D qh->qHead)->nextChain) =
=3D=3D NULL)
				qh->qTail =3D NULL;
			qh->qLen--;
		=09
			n0->nextChain =3D NULL;

      if (dst)
      {
			  trimmed =3D nTrim(dst, &n0, len);
			  if (dst)
				  dst +=3D trimmed;
			  len -=3D trimmed;
			  st +=3D trimmed;
      }
      else
      {
        len -=3D n0->chainLen;
        st +=3D n0->chainLen;
      }
      if (n0) nFreeChain(n0);
		}
	=09
		/* If more to go, trim from next chain. */
		if (len && qh->qHead) {
			/*=20
			 * XXX LONG CRITICAL SECTION!!!  Could we pop this =
off the queue,
			 * trim it, and then replace the remainder?  Do we =
need a semaphore?=20
			 */
      trimmed =3D nTrim(dst, &qh->qHead, len);
			st +=3D trimmed;

      // Update tail pointer!!! SERIOUS BUGFIX /ma...@mo... 2002.01.11
      if (qh->qLen =3D=3D 1)
      {
        // nTrim might return a new nBuf in qh->qHead
        // IF qLen =3D=3D 1 we have to update qTrail!!!=20
        qh->qTail =3D qh->qHead;
      }
		}
	}
=09
	return st;
}

[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
>
>