You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2010/07/15 20:04:47 UTC

RE: svn commit: r964487 - /subversion/trunk/subversion/libsvn_wc/wc_db.c


> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: donderdag 15 juli 2010 18:43
> To: commits@subversion.apache.org
> Subject: svn commit: r964487 -
> /subversion/trunk/subversion/libsvn_wc/wc_db.c
> 
> Author: philip
> Date: Thu Jul 15 16:42:54 2010
> New Revision: 964487
> 
> URL: http://svn.apache.org/viewvc?rev=964487&view=rev
> Log:
> GCC was warning "suggest parentheses around && within ||".
> 
> * subversion/libsvn_wc/wc_db.c
>   (svn_wc__db_wclock_obtain): Don't check for empty path as it will
>    be an ancestor.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_wc/wc_db.c
> 
> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_d
> b.c?rev=964487&r1=964486&r2=964487&view=diff
> =======================================================================
> =======
> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Jul 15 16:42:54
> 2010
> @@ -7814,10 +7814,9 @@ svn_wc__db_wclock_obtain(svn_wc__db_t *d
>            svn_wc__db_wclock_t lock = APR_ARRAY_IDX(wcroot-
> >owned_locks,
>                                                     i,
> svn_wc__db_wclock_t);
> 
> -          if ((*lock.relpath == '\0'
> -               || svn_relpath_is_ancestor(lock.relpath,
> baton.local_relpath)
> +          if (svn_relpath_is_ancestor(lock.relpath,
> baton.local_relpath)
>                && (lock.levels == -1
> -                  || (lock.levels + relpath_op_depth(lock.relpath)) >=
> depth)))
> +                  || (lock.levels + relpath_op_depth(lock.relpath)) >=
> depth))

I'm not sure if this doesn’t change the behavior. (Or: is "" the ancestor of "A" when using svn_relpath_is_ancestor()?)

If it isn't this breaks the locks on the working copy root.

	Bert

RE: svn commit: r964487 - /subversion/trunk/subversion/libsvn_wc/wc_db.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: donderdag 15 juli 2010 22:05
> To: dev@subversion.apache.org; commits@subversion.apache.org
> Subject: RE: svn commit: r964487 -
> /subversion/trunk/subversion/libsvn_wc/wc_db.c
> 
> 
> 
> > -----Original Message-----
> > From: philip@apache.org [mailto:philip@apache.org]
> > Sent: donderdag 15 juli 2010 18:43
> > To: commits@subversion.apache.org
> > Subject: svn commit: r964487 -
> > /subversion/trunk/subversion/libsvn_wc/wc_db.c
> >
> > Author: philip
> > Date: Thu Jul 15 16:42:54 2010
> > New Revision: 964487
> >
> > URL: http://svn.apache.org/viewvc?rev=964487&view=rev
> > Log:
> > GCC was warning "suggest parentheses around && within ||".
> >
> > * subversion/libsvn_wc/wc_db.c
> >   (svn_wc__db_wclock_obtain): Don't check for empty path as it will
> >    be an ancestor.
> >
> > Modified:
> >     subversion/trunk/subversion/libsvn_wc/wc_db.c
> >
> > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> > URL:
> >
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_d
> > b.c?rev=964487&r1=964486&r2=964487&view=diff
> >
> =======================================================================
> > =======
> > --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Jul 15 16:42:54
> > 2010
> > @@ -7814,10 +7814,9 @@ svn_wc__db_wclock_obtain(svn_wc__db_t *d
> >            svn_wc__db_wclock_t lock = APR_ARRAY_IDX(wcroot-
> > >owned_locks,
> >                                                     i,
> > svn_wc__db_wclock_t);
> >
> > -          if ((*lock.relpath == '\0'
> > -               || svn_relpath_is_ancestor(lock.relpath,
> > baton.local_relpath)
> > +          if (svn_relpath_is_ancestor(lock.relpath,
> > baton.local_relpath)
> >                && (lock.levels == -1
> > -                  || (lock.levels + relpath_op_depth(lock.relpath))
> >=
> > depth)))
> > +                  || (lock.levels + relpath_op_depth(lock.relpath))
> >=
> > depth))
> 
> I'm not sure if this doesn’t change the behavior. (Or: is "" the
> ancestor of "A" when using svn_relpath_is_ancestor()?)
> 
> If it isn't this breaks the locks on the working copy root.

Ok, just confirmed that this is safe. (I think there are a few other cases where I used this style to check, introduced in the same commit).

	Bert



RE: svn commit: r964487 - /subversion/trunk/subversion/libsvn_wc/wc_db.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: donderdag 15 juli 2010 22:05
> To: dev@subversion.apache.org; commits@subversion.apache.org
> Subject: RE: svn commit: r964487 -
> /subversion/trunk/subversion/libsvn_wc/wc_db.c
> 
> 
> 
> > -----Original Message-----
> > From: philip@apache.org [mailto:philip@apache.org]
> > Sent: donderdag 15 juli 2010 18:43
> > To: commits@subversion.apache.org
> > Subject: svn commit: r964487 -
> > /subversion/trunk/subversion/libsvn_wc/wc_db.c
> >
> > Author: philip
> > Date: Thu Jul 15 16:42:54 2010
> > New Revision: 964487
> >
> > URL: http://svn.apache.org/viewvc?rev=964487&view=rev
> > Log:
> > GCC was warning "suggest parentheses around && within ||".
> >
> > * subversion/libsvn_wc/wc_db.c
> >   (svn_wc__db_wclock_obtain): Don't check for empty path as it will
> >    be an ancestor.
> >
> > Modified:
> >     subversion/trunk/subversion/libsvn_wc/wc_db.c
> >
> > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> > URL:
> >
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_d
> > b.c?rev=964487&r1=964486&r2=964487&view=diff
> >
> =======================================================================
> > =======
> > --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Jul 15 16:42:54
> > 2010
> > @@ -7814,10 +7814,9 @@ svn_wc__db_wclock_obtain(svn_wc__db_t *d
> >            svn_wc__db_wclock_t lock = APR_ARRAY_IDX(wcroot-
> > >owned_locks,
> >                                                     i,
> > svn_wc__db_wclock_t);
> >
> > -          if ((*lock.relpath == '\0'
> > -               || svn_relpath_is_ancestor(lock.relpath,
> > baton.local_relpath)
> > +          if (svn_relpath_is_ancestor(lock.relpath,
> > baton.local_relpath)
> >                && (lock.levels == -1
> > -                  || (lock.levels + relpath_op_depth(lock.relpath))
> >=
> > depth)))
> > +                  || (lock.levels + relpath_op_depth(lock.relpath))
> >=
> > depth))
> 
> I'm not sure if this doesn’t change the behavior. (Or: is "" the
> ancestor of "A" when using svn_relpath_is_ancestor()?)
> 
> If it isn't this breaks the locks on the working copy root.

Ok, just confirmed that this is safe. (I think there are a few other cases where I used this style to check, introduced in the same commit).

	Bert


Re: svn commit: r964487 - /subversion/trunk/subversion/libsvn_wc/wc_db.c

Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:

> I'm not sure if this doesn’t change the behavior. (Or: is "" the
> ancestor of "A" when using svn_relpath_is_ancestor()?)

Yes, "" is an ancestor of "A".

-- 
Philip