On 2/20/07, Hyrum K. Wright <hyrum_wright@mail.utexas.edu> wrote:
> Ivan Zhakov wrote:
> > On 2/20/07, Daniel Rall <dlr@collab.net> wrote:
> >> On Mon, 19 Feb 2007, hwright@tigris.org wrote:
> >> ...
> >> > --- trunk/subversion/include/svn_opt.h (original)
> >> > +++ trunk/subversion/include/svn_opt.h Mon Feb 19 19:45:21 2007
> >> ...
> >> > -svn_error_t *
> >> > -svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
> >> > - svn_opt_revision_t *op_rev,
> >> > - svn_boolean_t is_url,
> >> > - svn_boolean_t notice_local_mods);
> >> ...
> >> > --- trunk/subversion/libsvn_subr/opt.c (original)
> >> > +++ trunk/subversion/libsvn_subr/opt.c Mon Feb 19 19:45:21 2007
> >> > @@ -610,6 +610,33 @@
> >> > }
> >> >
> >> >
> >> > +void
> >> > +svn_opt_resolve_revisions(svn_opt_revision_t *peg_rev,
> >> > + svn_opt_revision_t *op_rev,
> >> > + svn_boolean_t is_url,
> >> > + svn_boolean_t notice_local_mods)
> >> > +{
> >> > + if (peg_rev->kind == svn_opt_revision_unspecified)
> >> > + {
> >> > + if (is_url)
> >> > + {
> >> > + peg_rev->kind = svn_opt_revision_head;
> >> > + }
> >> > + else
> >> > + {
> >> > + if (notice_local_mods)
> >> > + peg_rev->kind = svn_opt_revision_working;
> >> > + else
> >> > + peg_rev->kind = svn_opt_revision_base;
> >> > + }
> >> > + }
> >> > +
> >> > + if (op_rev->kind == svn_opt_revision_unspecified)
> >> > + *op_rev = *peg_rev;
> >> > +
> >> > + return;
> >> > +}
> >> ...
> >>
> >> If we're really going to drop the return value of SVN_NO_ERROR (which
> >> we actually might not want to do in case a future variation on the
> >> implementation needs it), we should drop that return statement, too.
> >>
> > Also is there any guarantee that any implementation of this function
> > can be done without pool parameter in future? I'm not sure.
> >
>
> I'm trying to think of a scenario where we would need a pool parameter
> in the future. The only thing that I can think of would be if the
> svn_opt_revision_t structure was extended, and we needed to allocate
> something to finish the assignment. In this case, though, we'd need to
> rev the API anyway to use the new datatype.
>
> Are there other scenarios where we might need a pool parameter?
I don't have any specific scenarios now. But it can be just have to
call some APR function. I remember problems that we had with some
svn_path_* function, which doesn't have pool parameter and some day we
was needed call to apr_path_root().
So personally I prefer each *public* function have pool parameter and
return svn_error_t, unless we have strong objections.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Feb 21 09:16:35 2007