On Tue, Jun 18, 2013 at 05:08:55PM +0200, Bert Huijben wrote:
> > > err = svn_wc__upgrade_sdb(result_format, wcroot_abspath,
> > > - sdb, format, scratch_pool);
> > > + sdb, format, scratch_pool);
> > > +
> > > + if (bumped_format)
> > > + *bumped_format = (*result_format > format);
> >
> > Strictly speaking, I don't think we should modify *bumped_format here
> > in case there was an error. So I would expect this code to read like
> > this:
> >
> > if (err == SVN_NO_ERROR && bumped_format)
> > *bumped_format = (*result_format > format);
>
> There are no code paths where an error is not returned here (just cleaning
> up state), and once we return an error all output values are 'undefined'.
>
> So, yes it could be improved,
Ok. So I'll make this tweak if you don't mind.
> but there are far bigger issues in the upgrade code.
>
> (libsvn_client assumes upgrade is only used for <= 1.6 bumps and as such
> always upgrades externals from properties, ignoring the EXTERNALS table
> completely)
Well, that's an unrelated issue. I was just asking about the above
code snippet. I wasn't looking for any more worms in this can :)
Received on 2013-06-18 17:17:21 CEST