[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix crash in svn_wc_get_status_editor2

From: <kfogel_at_collab.net>
Date: 2005-11-29 22:54:16 CET

Julian Foad <julianfoad@btopenworld.com> writes:
> I've encountered a bug while reviewing Fabien Coelho's "svnversion to
> libsvn_wc" patch.
>
> * If @a traversal_info is non-null, then record pre-update traversal
> * state in it. (Caller should obtain @a traversal_info from
> * svn_wc_init_traversal_info().)
> ...
> svn_error_t *
> svn_wc_get_status_editor2 (const svn_delta_editor_t **editor,
> ...
> svn_wc_traversal_info_t *traversal_info, ...)
>
>
> It crashes if I pass NULL for traversal_info. The only caller we have
> so far doesn't pass null, but the new one will want to.
>
> This patch fixes it. Is it right?
>
> There is a possible alternative: Change the doc string to require that
> this parameter is provided. We might be able to justify that API
> change as a bug fix, because nobody can be using it much because it
> doesn't work, at least on WCs with unversioned items in them. But
> that's not really a safe attitude, and anyway it's ugly to require it
> from a caller that doesn't want it. (Revving the API isn't really a
> solution: we need to fix the bug in this version.)

The traversal_info is what allows an update to follow up to itself and
take care of changed svn:externals values (since when that property
changes, complex things must sometimes happen, such as the removal of
an old externals tree and the checking-out of a new one).

So allowing traversal_info to be NULL should be okay. After all, only
the caller could possibly use it, and if the caller knows it doesn't
need the traversal_info, who are we to argue?

Inspection of the pre-patch code (and post-patch, for that matter)
confirms that we were not using eb->traversal_info internally, we were
just setting values in it and handing it back to the caller.

Assuming this passes all tests, I think it's fine. Disentangling
those two concepts (the traversal_info and the "global" externals
hash) seems sensible anyway.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 30 00:19:42 2005

This is an archived mail posted to the Subversion Dev mailing list.