Re: [PATCH] Add svnrdump

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 9, 2010, 13:46 UTC
Message-ID: <20100709134600.GD12315@debian>
In-Reply-To: <002d01cb1e7f$e0ff03c0$a2fd0b40$@nl>

Hi,

Bert Huijben writes:

Show 11 quoted lines

> > +  /* Cleanup */
> > +  SVN_ERR(svn_io_file_close(hb->temp_file, hb->pool));
> > +  SVN_ERR(svn_stream_close(hb->temp_filestream));
> 
> The standard handler already closes the stream for you and if you don't
> disown the file on mapping, this also closes the file.
> 
> > +  svn_pool_destroy(hb->pool);
> 
> And as you clear the pool that contains the file and stream here, closing
> yourself is not necessary.

I realize this, but I closed the stream anyway to make debugging easier. The pool is only destroyed much later. Do you think this is bad policy?

-- Ram