On Sun, 2007-01-07 at 13:00 +0100, Erik Huelsmann wrote:
> On 1/7/07, Branko Čibej <brane@xbc.nu> wrote:
> > Bhuvaneswaran Arumugam wrote:
> > > Hello,
> > >
> > > Please find attached the patch. If this is a valid patch, i'd like to
> > > prepare similar clean-up patch for remaining files under
> > > subversion/libsvn_client directory.
> > >
> >
> > The whole point of using pools is that you don't have to deallocate
> > stuff (that's known to be size-bound). So no, this is not a valid patch.
> > It just messes up the code without actually gaining much.
>
> Yes it is. If we were not intending to destroy the memory, why use a
> subpool at all?
>
> Non-destruction of pools is valid when returning *in the error-case*,
> because we assume that the error will be handled and a parent pool
> will be cleared somewhere higher up the call chain. But for the
> non-error return, we neither assume nor actually do something like it.
Yep. To be precise, all these functions finally return SVN_NO_ERROR and
the subpool is destroyed before the return statement. But in certain
cases, the SVN_NO_ERROR value is returned from the middle of function
but the subpool is not destroyed. The patch exactly fixes this issue.
I think, as we already destroy the subpool in certain cases, it's good
to insure that we destroy it in all cases (at least to be consistent).
Comments are welcome.
--
Regards,
Bhuvaneswaran
Received on Mon Jan 8 04:53:00 2007