[reSIProcate] Bug in Data::append()

[Brian Cuthie]

Ok, I understand. I was apparently looking at old code (the 0.9.0
tarball), which clearly had a bug. But I've checked out head and I see
it's fixed. I should have done that before reporting the bug.
 
-brian


________________________________

	From: david Butcher [mailto:davidlbutcher at gmail.com] 
	Sent: Friday, March 24, 2006 10:21 PM
	To: Brian Cuthie
	Cc: Scott Godin; Byron Campen;
resiprocate-devel at list.sipfoundry.org
	Subject: Re: [reSIProcate] Bug in Data::append()
	
	
	We went to some effort to make sure the NULL is there to aid in
debugging.
	
	david
	
	
	On 3/22/06, Brian Cuthie < brian at systemix.com
<mailto:brian at systemix.com> > wrote: 


		It seems to me (and I'm admittedly a ReSIProcate
neophyte) that the 
		correct thing is to remove the statement in append()
that sets the NULL.
		If the buffer needed a NULL, then it would be there
already. And it
		looks to me like anything that wants a NULL accesses the
string through 
		c_str() anyway, which explicitly adds one.
		
		FWIW: I remove the statement in append() and I haven't
seen any problem
		yet.
		
		-brian
		
		
		> -----Original Message-----
		> From: Scott Godin [mailto: slgodin at icescape.com]
		> Sent: Wednesday, March 22, 2006 5:52 PM
		> To: Byron Campen; Brian Cuthie
		> Cc: resiprocate-devel at list.sipfoundry.org 
		> Subject: RE: [reSIProcate] Bug in Data::append()
		>
		> Actually it looks like there is a problem if mCapacity
==
		> mSize + len and mMime != Share.  In this case by then
end of
		> the function mCapacity == mSize and mBuf[mSize] is out
of range. 
		>
		> Perhaps the initial if statement should read:
		> if (mCapacity <= mSize + len)
		>
		> Scott
		>
		> -----Original Message-----
		> From: resiprocate-devel-bounces at list.sipfoundry.org
		> [mailto:resiprocate-devel-bounces at list.sipfoundry.org]
On
		> Behalf Of Byron Campen
		> Sent: Wednesday, March 22, 2006 5:45 PM 
		> To: Brian Cuthie
		> Cc: resiprocate-devel at list.sipfoundry.org
		> Subject: Re: [reSIProcate] Bug in Data::append()
		>
		>       Actually, that buffer has size mSize+1, so that
null 
		> termination is not causing memory corruption. But,
your
		> heap-corruption issue is still real, so I ask what
version of
		> resiprocate you are using.
		>
		> Best regards,
		> Byron Campen
		>
		>
		> >
		> > Hello,
		> >
		> > I'm new to this list, and to using the Reciprocate
stack.
		> However, I
		> > think I found a bug and wanted to hear what people
think.
		> I'm sure I'm
		> > doing this wrong, so please don't be shy about
helping me
		> understand
		> > the correct procedure.
		> >
		> > Background:
		> >
		> > While testing the stack under Windows, I had
problems with a heap 
		> > corruption when writing debug log messages using the
		> VSDebugWindow log
		> > type.
		> >
		> > BUG:
		> >
		> > The following code in Data::append() appears to be
in 
		> error. The NULL
		> > is written past the end of the buffer, and
furthermore I
		> think it is
		> > unnecessary, since c_str() adds a NULL.
		> >
		> > Data&
		> > Data::append(const char* str, size_type len) { 
		> >    assert(str);
		> >    if (mCapacity < mSize + len)
		> >    {
		> >       // .dlb. pad for future growth?
		> >       resize(((mSize + len +16)*3)/2, true);
		> >    } 
		> >    else
		> >    {
		> >       if (mMine == Share)
		> >       {
		> >          char *oldBuf = mBuf;
		> >          mCapacity = mSize + len;
		> >          mBuf = new char[mSize + len]; 
		> >          memcpy(mBuf, oldBuf, mSize);
		> >          mMine = Take;
		> >       }
		> >    }
		> >
		> >    // could conceivably overlap
		> >    memmove(mBuf + mSize, str, len); 
		> >    mSize += len;
		> >    mBuf[mSize] = 0;   // ERROR
		> >
		> >    return *this;
		> > }
		> >
		> > Finally, there appears to be a related bug fix to
Data::c_str() 
		> > (#5408).
		> > I'm wondering if someone was tracking this same
problem and
		> tried to
		> > fix it there, since resize() will copy the buffer
anyway,
		> making the
		> > check in c_str() unecessary. 
		> >
		> > Cheers,
		> >
		> > Brian
		> >
		> > _______________________________________________
		> > resiprocate-devel mailing list
		> > resiprocate-devel at list.sipfoundry.org
		> >
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
		>
		>
		_______________________________________________ 
		resiprocate-devel mailing list
		resiprocate-devel at list.sipfoundry.org
	
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel 
		


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20060324/5d2f1877/attachment.htm>