Malcolm Rowe <malcolm-svn-dev@farside.org.uk> writes:
> On Mon, Jan 30, 2006 at 08:32:59AM -0600, lundblad@tigris.org wrote:
> > --- trunk/subversion/libsvn_wc/status.c (original)
> > +++ trunk/subversion/libsvn_wc/status.c Mon Jan 30 08:32:54 2006
> > @@ -1155,13 +1155,14 @@
> > /* Order is important here. We can't depend on parent_status->entry
> > being non-NULL until after we've checked all the conditions that
> > might indicate that the parent is unversioned ("unversioned" for
> > - our purposes includes being an external). */
> > + our purposes includes being an external or ignored item). */
> > if (parent_status
> > && (parent_status->text_status != svn_wc_status_unversioned)
> > && (parent_status->text_status != svn_wc_status_deleted)
> > && (parent_status->text_status != svn_wc_status_missing)
> > && (parent_status->text_status != svn_wc_status_obstructed)
> > && (parent_status->text_status != svn_wc_status_external)
> > + && (parent_status->text_status != svn_wc_status_ignored)
> > && (parent_status->entry->kind == svn_node_dir)
> > && (eb->descend || (! pb)))
> > {
>
> Any reason that condition can't just be
>
> if (parent_status
> && parent_status->entry
> && parent_status->entry->kind == svn_node_dir ...)
>
> (The comment seems to suggest we're just checking a list of conditions
> that determine whether ->entry is non-NULL: why can't we just check
> ->entry itself?)
>
> Seems simpler and more futureproof, if it works.
(Speaking only for myself here, not lundblad, of course.)
Sure that would be futureproof, but it could mask unexpected error
situations. If parent_status->entry were NULL, we'd just blithely
skip along, without having any idea why it was NULL or what that might
signify. I feel like it's better to enumerate the expected
conditions, because that means we understand what's going on in the
code (or, as in the current case, it alerts us to when we don't
understand it well enough).
If we could guarantee to ourselves that *all* possible reasons why
parent_status->entry might be NULL are known and acceptable, then we
could do the shorter check. But looking at the code, I find I can't
make that guarantee to myself. Can you?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jan 30 17:19:22 2006