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

Re: svn commit: r1764447 - in /subversion/trunk/subversion: libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c

From: Stefan Sperling <stsp_at_apache.org>
Date: Mon, 24 Oct 2016 11:59:29 +0200

On Mon, Oct 24, 2016 at 11:18:51AM +0200, Bert Huijben wrote:
>
>
> > -----Original Message-----
> > From: Stefan Sperling [mailto:stsp_at_apache.org]
> > Sent: maandag 24 oktober 2016 11:08
> > To: dev_at_subversion.apache.org
> > Subject: Re: svn commit: r1764447 - in /subversion/trunk/subversion:
> > libsvn_client/conflicts.c tests/libsvn_client/conflicts-test.c
> >
> > On Sun, Oct 23, 2016 at 03:31:22PM -0400, James McCoy wrote:
> > > On Wed, Oct 12, 2016 at 12:34:19PM -0000, stsp_at_apache.org wrote:
> > > > Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
> > > > URL:
> >
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/confl
> ic
> > ts.c?rev=1764447&r1=1764446&r2=1764447&view=diff
> > > >
> > ================================================================
> > ==============
> > > > --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
> > > > +++ subversion/trunk/subversion/libsvn_client/conflicts.c Wed Oct 12
> > 12:34:18 2016
> > > > @@ -5666,6 +5726,18 @@ diff_file_added(const char *relpath,
> > > > FALSE, FALSE, scratch_pool));
> > > > SVN_ERR(svn_io_check_path(local_abspath, &on_disk_kind,
> > scratch_pool));
> > > >
> > > > + if (db_kind == svn_node_file && db_kind == svn_node_file)
> > >
> > > Coverity noticed both sides of the && are the same. Should one side be
> > > "on_disk_kind == svn_node_file" instead?
> > >
> >
> > Yes indeed, it should. Thanks! I'll fix this when I find time
> > if it's not already been fixed by then.
>
> Be careful when committing this as an 'obvious fix'. I've seen a very
> similar problem in the conflict code in the past that has very explicit
> tests in our testsuite that verify the current behavior. It is very well
> possible that this is just the same code moved to a different location.
>
> If this is that specific case the problem should be fixed, but it won't be
> an obvious fix... more a case of careful review of all the cases that
> trigger this code and then updating expected results.
>
> Bert

Yeah, that's why I didn't commit it myself on the spot.
It needs at least one run through the test suite.
Received on 2016-10-24 12:00:12 CEST

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