cmpilato_at_tigris.org writes:
> Log:
> Fix SEGFAULT introduced in r32545.
>
> * subversion/libsvn_fs_fs/tree.c
> (fs_node_origin_rev): Avoid dup'ing a NULL pointer.
>
> --- trunk/subversion/libsvn_fs_fs/tree.c
> +++ trunk/subversion/libsvn_fs_fs/tree.c
> @@ -3084,7 +3084,7 @@ fs_node_origin_rev(svn_revnum_t *revisio
> (which is allocated in SUBPOOL ... maybe). */
> svn_pool_clear(predidpool);
> SVN_ERR(svn_fs_fs__dag_get_predecessor_id(&pred_id, node, subpool));
> - pred_id = svn_fs_fs__id_copy(pred_id, predidpool);
> + pred_id = pred_id ? svn_fs_fs__id_copy(pred_id, predidpool) : NULL;
> }
Not a big deal, but that way of expressing the logic felt a little odd
to me. Instead, wouldn't
if (pred_id)
pred_id = svn_fs_fs__id_copy(pred_id, predidpool);
...be the usual way to do this?
The "foo ? bar : baz;" syntax implies that 'baz' part makes a
difference. But here it has no effect: if pred_id is NULL, then set it
to NULL :-). So I think 'if' would be more natural for cases like this.
Very minor readability boost; I don't think it's worth the code/annotate
churn to tweak it now, just pointing it out for next time.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-20 17:31:23 CEST