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 :)
