Xapian devel - Apr 2007 - Re: [Xapian-commits] 8107: trunk/xapian-core/ trunk/xapian-core/backends/

olly wrote:
> Log message (7 lines):
> backends/database.cc: Database::Internal can't call the
> PostingIterator(PostingIterator::Internal*) ctor (at least under
> g++ 3.3.5) because it isn't a friend (only class Database is).
For the record, Mark just reported this to me under windows so it was a 
problem there too, but it does work under GCC 4.1.  No idea which 
compiler is "correct", but that hardly matters...
> Can't seem to forward define Database::Internal to make
> Database::Internal a friend so just use LeafPostList directly
> as that seems less bad than pulling in the whole of database.h
> or making PostingIterator::internal public.
Only problem with this patch is that it looks like it leaks the 
LeafPostList if an exception is thrown by one of the other methods (such 
as delete_document()).  Actually, does the postlist get deleted at all?

I've changed this to wrap the postlist in a RefCntPtr which should fix 
the leak issue.

-- 
Richard

On Thu, Apr 05, 2007 at 12:20:57PM +0100, Richard Boulton
wrote:
> olly wrote:
> >Log message (7 lines):
> >backends/database.cc: Database::Internal can't call the
> >PostingIterator(PostingIterator::Internal*) ctor (at least under
> >g++ 3.3.5) because it isn't a friend (only class Database is).
> 
> For the record, Mark just reported this to me under windows so it was a 
> problem there too
With MSVC or GCC (or something else)?
> but it does work under GCC 4.1.  No idea which compiler is
"correct",
> but that hardly matters...
My money is on 4.1 being correct, but as you say it hardly matters -it's
not reasonable to drop support for compilers in common use if there's an
sane workaround.
> >Can't seem to forward define Database::Internal to make
> >Database::Internal a friend so just use LeafPostList directly
> >as that seems less bad than pulling in the whole of database.h
> >or making PostingIterator::internal public.
> 
> Only problem with this patch is that it looks like it leaks the 
> LeafPostList if an exception is thrown by one of the other methods (such 
> as delete_document()).  Actually, does the postlist get deleted at all?
No - I just ran "apitest" directly before checking in so missed this,
but valgrind picked it up for me when I tried to test the next part.
But you beat me to a fix.
> I've changed this to wrap the postlist in a RefCntPtr which should fix 
> the leak issue.
Yeah, that's reasonable enough.  Perhaps we should profile AutoPtr vs
RefCntPtr to see if one of them is better for such situations - I've
wondered before.

Cheers,
    Olly