RE: [reSIProcate] Bug in Data::append()
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
We went to some effort to make sure the NULL is there to aid in
debugging.
david
On 3/22/06, Brian
Cuthie < brian@xxxxxxxxxxxx>
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@xxxxxxxxxxxx]
> Sent:
Wednesday, March 22, 2006 5:52 PM
> To: Byron Campen; Brian
Cuthie
> Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxx
>
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx]
On
> Behalf Of Byron Campen
> Sent: Wednesday, March 22, 2006
5:45 PM
> To: Brian Cuthie
> Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
>
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@xxxxxxxxxxxxxxxxxxx
>
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>
>
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxxxxxx
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel