[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1493703 - in /subversion/trunk/subversion/libsvn_wc: upgrade.c wc_db.c wc_db.h

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 18 Jun 2013 17:16:43 +0200

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

This is an archived mail posted to the Subversion Dev mailing list.