Re: [CLucene-dev] Memory fixes to clucene

[<rg...@sd...>]

>>>>> "Albert" == Albert Vila Puig <av...@im...> writes:
Albert> 
Albert> Hi all,
Albert>     I downloaded the patch, compiled and installed. I add another patch, the
Albert>     StringBuffer.cpp class has a few problems.
Albert> 
Albert>     1- The prepend method
Albert>         //if ( len+sl+1 > bufferLength )
Albert>         //    growBuffer ( );
Albert>         while(len+sl+1>bufferLength)
Albert>             growBuffer();

It is correct that this should be a while instead of an if.

Albert>     2- The clear method
Albert>         delete buffer;
Albert>         buffer = new char_t[LUCENE_STREAM_BUFFER_SIZE];
Albert>         bufferLength = LUCENE_STREAM_BUFFER_SIZE;
Albert>         len = 0;

I have not had to modify this particular file in my patches, I am
working from exactly what is in the CVS as far as this file goes.

However, I think I will have to carefully work through it, and I would
like to point out the problems so you guys can fix it up if you get to
it before me:

1) Buffer is allocated with the syntax "buffer = new
   char_t[LUCENE_STREAM_BUFFER_SIZE]".  This means that buffer MUST be
   deallocated with "delete[] buffer" not "delete buffer".

2) In this constructor

StringBuffer::StringBuffer(const char_t* value):
	buffer (stringDuplicate(value)),
	bufferLength(stringLength(value)),
	len(stringLength(value))
{
}

buffer is allocated by doing a stringDuplicate() of the value, which
is the same as standard C library strdup() (at least on linux), which
uses malloc() to allocate, not new / new[].  So the buffer in that
case MUST be de-allocated with free() not delete or delete[].  But of
course once you get to the functions were you are deallocating, you
don't have anyway of telling if you got the buffer pointer from one
constructor or another, so you can't figure out which way to
deallocate.

For a class like this it is a good idea to make sure the variable
buffer always is allocated and deallocated by the same machanism,
instead of keeping flags to indicate it, or something like that.
Another thing that can get you in trouble is loosing track of whether
the memory has been allocated by the function that called you, and
should be de-allocated by that function, or you allocated it and
should de-allocate it.

Sometimes people working on a largish thing like CLucene make a global
decision that "all strings will be C-style malloc/free char*'s" or
"all strings will be allocated with new[]" or use the C++ basic string
everywhere.  As we get some of this sorted out we may find ourselves
settling into some similar policy.

Albert>     Now, when I execute my demo application, it just crash by
Albert>     segmentation fault after the _cout << "start indexing" <<
Albert>     endln; line.

Albert> If I add the _cout << "test" line after the error line, the
Albert> program terminates by segmentation fault as well.
Albert> 
Albert> 
Albert> Any suggestions?
Albert> 
Albert> Thanks.

Are you set up so that you can run this program in gdb or valgrind ?
If you can compile CLucene with debugging flags, then you should be
able to use gdb to see what specific pointer it is that is pointing
outside the program's memory (that's what a segmentation fault usually
is).  If you run it in valgrind, it should give you some errors to
work on, and the last one listed is probably causing the immediate
problem.

I can assist you in using gdb and valgrind if you are working in a
unix environment.

--Rob