> -----Original Message-----
> From: 'Stefan Sperling' [mailto:stsp_at_elego.de]
> Sent: zondag 1 augustus 2010 22:50
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org
> Subject: Re: svn commit: r980811 - in
> /subversion/trunk/subversion/libsvn_fs_fs: fs.h fs_fs.c
>
> On Sun, Aug 01, 2010 at 03:50:49PM +0200, Bert Huijben wrote:
> > Yes, her you do, but before that there are a few SVN_ERR() statements
> that
> > can return WITHOUT wrapping the error.
> >
> > + SVN_ERR(svn_dirent_get_absolute(&src_abspath,
> src_path,
> > pool));
> > + SVN_ERR(svn_dirent_get_absolute(&dst_abspath,
> dst_path,
> > pool));
> >
> > So these two lines (from your original patch), leak the error if they
> return
> > an error message.
>
> Ah, yeah. I see. What's the best way to handle this?
> Is simply assigning err to a temporary err2 variable safe?
> I would not expect so.
>
> Maybe instead of wrapping the error, we should just create a new one?
Looking a bit further: Do we really need to take the absolute path for the
error message? (What do we in similar code paths?)
The easiest way to fix this would be to just use the caller provided paths
in the error message. That would immediately fix the error leak.
In this case you would need to use src_path for the error message anyway if
the svn_dirent_get_absolute() fails.
Bert
Received on 2010-08-01 22:57:27 CEST