Greg Stein <gstein@lyra.org> writes:
> > - /* If both REVISION and TM are specified, this is an error.
> > - They mostly likely contradict one another. */
> > - if ((revision != SVN_INVALID_REVNUM) && tm)
> > - return
> > - svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, 0, NULL, pool,
> > - "Cannot specify _both_ revision and time.");
>
> Who is performing this consistency check now?
>
> svn_client__get_revision_number ?
>
> Does that function have enough context to do the right checks?
That particular consistency check has become meaningless for many
libsvn_client functions, since the information they're receiving now
*can't* be inconsistent (it's one var now, not two).
There are some universal checks that can go into the main argument
parsing (the above may be one, but not sure yet), and various
function-specific checks that should go into libsvn_client...
> > + /* Sanity check. */
> > + if ((start->kind == svn_client_revision_unspecified)
> > + || (end->kind == svn_client_revision_unspecified))
> > + {
> > + return svn_error_create
> > + (SVN_ERR_CLIENT_BAD_REVISION, 0, NULL, pool,
> > + "svn_client_diff: caller failed to specify any revisions");
> > + }
>
> Why does this one do it, yet the others don't?
... such as this one. :-)
Yeah, in general, each client interface will have to specify its
revision rules. The rules for svn_client_diff() aren't completely
decided yet, because I'm sweeping over all the changed client
interfaces to see if maybe there's some simple rule that will cover
most/all of them...
(IOW, still thinking.)
> Repeat after me: const. const. const. const....
>
> :-)
>
> However, I would simply recommend making those embedded structures. I'm not
> sure that I see a reason to allocate that stuff on the heap. Especially when
> the structure can represent "unspecified" explicitly (rather than using a
> NULL pointer to say the same).
Yeah, embedding is better, I'll just do that.
Thanks for the review, Greg!
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:37:03 2006