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 2010/04/09 22:15:21 UTC
Re: svn commit: r932635 - /subversion/trunk/subversion/libsvn_repos/reporter.c
On 04/09/2010 03:08 PM, stsp@apache.org wrote:
> Author: stsp
> Date: Fri Apr 9 22:08:40 2010
> New Revision: 932635
>
> URL: http://svn.apache.org/viewvc?rev=932635&view=rev
> Log:
> * subversion/libsvn_repos/reporter.c
> (update_entry): Style nit: Replace "if (a) b = TRUE;" with "b = (a);"
> - else if (distance != -1 || b->ignore_ancestry)
> - related = TRUE;
> +
> + related = (distance != -1 || b->ignore_ancestry);
Why not then also remove the related = FALSE just above this code as
it's redundant.
Blair
Re: svn commit: r932635 - /subversion/trunk/subversion/libsvn_repos/reporter.c
Posted by Blair Zajac <bl...@orcaware.com>.
On 04/09/2010 03:33 PM, Stefan Sperling wrote:
> On Fri, Apr 09, 2010 at 03:15:21PM -0700, Blair Zajac wrote:
>> On 04/09/2010 03:08 PM, stsp@apache.org wrote:
>>> Author: stsp
>>> Date: Fri Apr 9 22:08:40 2010
>>> New Revision: 932635
>>>
>>> URL: http://svn.apache.org/viewvc?rev=932635&view=rev
>>> Log:
>>> * subversion/libsvn_repos/reporter.c
>>> (update_entry): Style nit: Replace "if (a) b = TRUE;" with "b = (a);"
>>
>>> - else if (distance != -1 || b->ignore_ancestry)
>>> - related = TRUE;
>>> +
>>> + related = (distance != -1 || b->ignore_ancestry);
>>
>> Why not then also remove the related = FALSE just above this code as
>> it's redundant.
>
> Because the assignment happens inside an if-statement.
> If we don't enter the if-statement, 'related' may end
> up being uninitialised.
>
> Here's the full context:
>
> related = FALSE;
> if (s_entry&& t_entry&& s_entry->kind == t_entry->kind)
> {
> distance = svn_fs_compare_ids(s_entry->id, t_entry->id);
> if (distance == 0&& !any_path_info(b, e_path)
> && (!info || (!info->start_empty&& !info->lock_token))
> && (requested_depth<= wc_depth || t_entry->kind == svn_node_file))
> return SVN_NO_ERROR;
>
> related = (distance != -1 || b->ignore_ancestry);
> }
Oh right, too quick on the review :)
Blair
Re: svn commit: r932635 -
/subversion/trunk/subversion/libsvn_repos/reporter.c
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Apr 09, 2010 at 03:15:21PM -0700, Blair Zajac wrote:
> On 04/09/2010 03:08 PM, stsp@apache.org wrote:
> >Author: stsp
> >Date: Fri Apr 9 22:08:40 2010
> >New Revision: 932635
> >
> >URL: http://svn.apache.org/viewvc?rev=932635&view=rev
> >Log:
> >* subversion/libsvn_repos/reporter.c
> > (update_entry): Style nit: Replace "if (a) b = TRUE;" with "b = (a);"
>
> >- else if (distance != -1 || b->ignore_ancestry)
> >- related = TRUE;
> >+
> >+ related = (distance != -1 || b->ignore_ancestry);
>
> Why not then also remove the related = FALSE just above this code as
> it's redundant.
Because the assignment happens inside an if-statement.
If we don't enter the if-statement, 'related' may end
up being uninitialised.
Here's the full context:
related = FALSE;
if (s_entry && t_entry && s_entry->kind == t_entry->kind)
{
distance = svn_fs_compare_ids(s_entry->id, t_entry->id);
if (distance == 0 && !any_path_info(b, e_path)
&& (!info || (!info->start_empty && !info->lock_token))
&& (requested_depth <= wc_depth || t_entry->kind == svn_node_file))
return SVN_NO_ERROR;
related = (distance != -1 || b->ignore_ancestry);
}
Stefan