> -----Original Message-----
> From: 'Stefan Sperling' [mailto:s...@elego.de]
> Sent: zondag 1 augustus 2010 12:48
> To: Bert Huijben
> Cc: dev@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 10:39:04AM +0200, Bert Huijben wrote:
> > > +          if (APR_STATUS_IS_ENOENT(err->apr_err))
> > > +            {
> > > +              /* 1.6.0 to 1.6.11 did not copy the configuration
> file
> > > during
> > > +               * hotcopy. So if we're hotcopying a repository
> which
> > > has been
> > > +               * created as a hotcopy itself, it's possible that
> > > fsfs.conf
> > > +               * does not exist. Ask the user to re-create it.
> > > +               *
> > > +               * ### It would be nice to make this a non-fatal
> error,
> > > +               * ### but this function does not get an svn_fs_t
> object
> > > +               * ### so we have no way of just printing a warning
> via
> > > +               * ### the fs->warning() callback. */
> > > +
> > > +              const char *msg;
> > > +              const char *src_abspath;
> > > +              const char *dst_abspath;
> > > +              const char *config_relpath;
> > > +
> >
> > You leak the error in err here.
> 
> No, because we quick_wrap it later:
> 
> > > +              return svn_error_return(svn_error_quick_wrap(err,


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.


        Bert

> msg));
>                                             ^^^^^^^^^^^^^^^^^^^^^^^^
> > > +            }
> > > +          else
> > > +            return svn_error_return(err);
> > > +        }
> > > +    }
> > > +

Reply via email to