Function file_to_string() throws away result

[C.vanReeuwijk]

It took me some time to convince myself this really is a bug, since it essentially disables all indexing of plain text and html (!!) files. Somebody should have noted this before. Still, changing this works for me...

In the function file_to_string()' in the file omindex.cc', a string variable `out' is filled with the contents of a file. However, after that nothing is done with the variable, since the empty string is returned. I think that is incorrect, and that at the last line of the function, `return "";' should be replaced by `return out;'.

Moreover, `file_to_string()' is used at two places, where a test

if( !out.empty() ){ <abort> }

is done. I think the `!' should be removed to restore the original intention of the code.

Finally, the C++ forbids mixing of new' and malloc' allocation, so the malloc' in file_to_string()' should be replaced by a `new []' expression.

[james]

Fixed the basic bug - file_to_string() now returns the contents of the file. Also made it not leak file descriptors (and hence memory) on empty files.

Left the malloc() vs new[] change for now, since I can't find any references that say it's /illegal/ to mix malloc and new; indeed, it can't possibly be illegal, because a C++ program might need to access a legacy C library. It might be illegal to mix them within a single code unit (still doubtful), but we're not directly doing that, even. Stroustroup and Meyers both dissuade you from using malloc/free and friends in C++, but that's a different matter entirely. (And if we fix this, we should probably make it throw an exception on bad file, so we don't need the empty file -> " " hack.)

[james]

From Kees (bug submittor):

I'm sorry, you are correct. What is illegal is doing a free() on a new-ed block and vice versa, not using both new/delete and malloc/free on separate blocks.

Closing this bug accordingly.

[olly]

Hmm, sorry about the untested code slipping into a release. I'm trying to juggle too many things at the moment I think. Anyway...

You're not allowed to allocate a block with malloc and release it with delete, or any other mismatched combination (new/delete[] and new[]/delete are also mismatched). But there's nothing illegal about using malloc and new for separate allocations in C++. Some would argue it's bad style perhaps.

But allocating a block for the whole file size, then copying it into a string then deleting the original block is very likely to fragment the heap, so I've reworked to read the file in chunks, and also used exceptions to report read errors, as James suggests.