Julian Foad <firstname.lastname@example.org> 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.
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Wed Nov 30 00:19:42 2005