You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2008/07/01 13:50:54 UTC
Re: svn commit: r31941 - branches/issue-2843-dev/subversion/libsvn_wc
firemeteor@tigris.org wrote:
> Author: firemeteor
> Date: Tue Jul 1 06:31:16 2008
> New Revision: 31941
>
> Log:
> On the issue-2843-dev branch.
>
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc_remove_from_revision_control): Fix a problem introduced in merge.
>
> * subversion/libsvn_wc/entries.c
> (read_entries): Verify depth value as suggested by Karl.
>
> Modified:
> branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c
> branches/issue-2843-dev/subversion/libsvn_wc/entries.c
>
> Modified: branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.collab.net/viewvc/svn/branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c?pathrev=31941&r1=31940&r2=31941
> ==============================================================================
> --- branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c Tue Jul 1 02:43:13 2008 (r31940)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/adm_ops.c Tue Jul 1 06:31:16 2008 (r31941)
> @@ -2569,7 +2569,7 @@ svn_wc_remove_from_revision_control(svn_
> the entry in the parent directory should be preserved
> for bookkeeping purpose. This only happens when the
> function is called by svn_wc_crop_tree(). */
> - dir_entry = apr_hash_get(entries, base_name,
> + dir_entry = apr_hash_get(parent_entries, base_name,
> APR_HASH_KEY_STRING);
> if (dir_entry->depth != svn_depth_exclude)
> {
>
> Modified: branches/issue-2843-dev/subversion/libsvn_wc/entries.c
> URL: http://svn.collab.net/viewvc/svn/branches/issue-2843-dev/subversion/libsvn_wc/entries.c?pathrev=31941&r1=31940&r2=31941
> ==============================================================================
> --- branches/issue-2843-dev/subversion/libsvn_wc/entries.c Tue Jul 1 02:43:13 2008 (r31940)
> +++ branches/issue-2843-dev/subversion/libsvn_wc/entries.c Tue Jul 1 06:31:16 2008 (r31941)
> @@ -492,9 +492,28 @@ read_entry(svn_wc_entry_t **new_entry,
> const char *result;
> SVN_ERR(read_val(&result, buf, end));
> if (result)
> - entry->depth = svn_depth_from_word(result);
> + {
> + svn_boolean_t invalid;
> + svn_boolean_t is_this_dir;
> +
> + entry->depth = svn_depth_from_word(result);
> +
> + /* Verify the depth value:
> + THIS_DIR should not have an excluded value and SUB_DIR should only
> + have excluded value. Remember that infinity value is not stored and
> + should not show up here. Otherwise, something bad may have
> + happened. However, infinity value itself will always be okay. */
> + is_this_dir = !name;
> + invalid = is_this_dir ^ (entry->depth != svn_depth_exclude);
I'm pretty sure as a style issue we don't use bitwise operations in boolean
operations here. If you could rewrite that in normal !, && and || that would be
clearer.
Regards,
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31941 - branches/issue-2843-dev/subversion/libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
"Rui, Guo" <ti...@mail.ustc.edu.cn> writes:
>> invalid = is_this_dir != (entry->depth != svn_depth_exclude);
> Thanks. This looks great!
Heh, Lorenz said exactly what I was going to say :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31941 -
branches/issue-2843-dev/subversion/libsvn_wc
Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
On Wed, Jul 02, 2008 at 08:42:20AM +0000, Lorenz wrote:
> >>> + invalid = is_this_dir ^ (entry->depth != svn_depth_exclude);
> >>
> >> I'm pretty sure as a style issue we don't use bitwise operations in
> >> boolean operations here. If you could rewrite that in normal !, && and
> >> || that would be clearer.
> >The logic here is indeed XOR. What if I change this to the following?
> > invalid = (is_this_dir ^ (entry->depth != svn_depth_exclude)) == 1;
>
> just use != as the boolean equivalent
>
> invalid = is_this_dir != (entry->depth != svn_depth_exclude);
Thanks. This looks great!
Rui
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31941 - branches/issue-2843-dev/subversion/libsvn_wc
Posted by Lorenz <lo...@yahoo.com>.
>>> + invalid = is_this_dir ^ (entry->depth != svn_depth_exclude);
>>
>> I'm pretty sure as a style issue we don't use bitwise operations in
>> boolean operations here. If you could rewrite that in normal !, && and
>> || that would be clearer.
>The logic here is indeed XOR. What if I change this to the following?
> invalid = (is_this_dir ^ (entry->depth != svn_depth_exclude)) == 1;
just use != as the boolean equivalent
invalid = is_this_dir != (entry->depth != svn_depth_exclude);
--
Lorenz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31941 -
branches/issue-2843-dev/subversion/libsvn_wc
Posted by "Rui, Guo" <ti...@mail.ustc.edu.cn>.
On Tue, Jul 01, 2008 at 06:50:54AM -0700, Blair Zajac wrote:
>> Modified: branches/issue-2843-dev/subversion/libsvn_wc/entries.c
>> URL: http://svn.collab.net/viewvc/svn/branches/issue-2843-dev/subversion/libsvn_wc/entries.c?pathrev=31941&r1=31940&r2=31941
>> ==============================================================================
>> --- branches/issue-2843-dev/subversion/libsvn_wc/entries.c Tue Jul 1 02:43:13 2008 (r31940)
>> +++ branches/issue-2843-dev/subversion/libsvn_wc/entries.c Tue Jul 1 06:31:16 2008 (r31941)
>> @@ -492,9 +492,28 @@ read_entry(svn_wc_entry_t **new_entry,
>> const char *result;
>> SVN_ERR(read_val(&result, buf, end));
>> if (result)
>> - entry->depth = svn_depth_from_word(result);
>> + {
>> + svn_boolean_t invalid;
>> + svn_boolean_t is_this_dir;
>> +
>> + entry->depth = svn_depth_from_word(result);
>> +
>> + /* Verify the depth value: + THIS_DIR should not
>> have an excluded value and SUB_DIR should only
>> + have excluded value. Remember that infinity value is not stored and
>> + should not show up here. Otherwise, something bad may have
>> + happened. However, infinity value itself will always be okay. */
>> + is_this_dir = !name;
>> + invalid = is_this_dir ^ (entry->depth != svn_depth_exclude);
>
> I'm pretty sure as a style issue we don't use bitwise operations in
> boolean operations here. If you could rewrite that in normal !, && and
> || that would be clearer.
The logic here is indeed XOR. What if I change this to the following?
invalid = (is_this_dir ^ (entry->depth != svn_depth_exclude)) == 1;
Regards,
Rui
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org