What he said.
Greg Stein <gstein@lyra.org> writes:
> On Thu, Feb 08, 2001 at 11:37:48AM -0500, Kevin Pilch-Bisson wrote:
> >...
> > Here are the prototypes I have come up with so far:
> > svn_error_t *
> > svn_?_condense_targets(svn_string_t ***pcondensed_targets,
> > int *pnum_condensed_targets,
> > const svn_string_t **targets,
> > int num_targets,
> > apr_pool_t *pool);
> >
> > svn_error_t *
> > svn_?_get_basepath(svn_string_t **pbasepath,
> > const svn_string_t **targets,
> > int num_targets,
> > apr_pool_t *pool);
> >
> > Now for questions and notes:
> >
> > 1) What library should they be part of?
>
> subr. I see them as part of path processing.
>
> > 2) They currently return svn_error_t *, but nothing they call generates
> > an error, so they could just as well return results directly.
>
> Yup. Return an apr_array_header_t*, containing the svn_string_t* ptrs.
>
> > 3) In condense_targets, I can calculate the basepath as a side-effect,
> > although it would be (slightly) less efficient than doing it after,
> > depending on the order of targets.
>
> Optional return value from condense_targets. If pbasepath != NULL, then you
> get the value returned to you.
>
> > 4) I have been creating a subpool of pool for temporary allocation, and
> > apr_pool_destroy()'ing it at the end of the functions, is this
> > appropriate?
>
> I'd say no.
>
> SVN already spends too much time trying to be "nifty" with its allocations.
> As a result, we end up with things in the wrong pools, and then we have
> lifetime mismatches.
>
> Put your allocations and temporary allocations directly into the provided
> pool. If the *caller* ends up calling these things way too often (e.g.
> unbounded due to be proportional to, say, a tree traversal), then the caller
> can create subpools. Or we can revise the function.
>
> Trying to optimize memory usage now (read: prematurely) can only lead to
> problems.
>
> > 5) I could also modify the targets array and num targets directly. Thus
>
> I'm not sure about this one. I'd initially say "no", but defer to Ben on
> whether the original set is needed.
>
> >...
> > Problems with the algo's:
> > 1) Won't work if some paths are given relatively and some absolutely.
> > 2) No garantee that basepath is in a working copy
> > 3) No garantee that basepath is a directory if given only a single
> > file as a target.
> >
> > What do you all think?
>
> (1) must be fixed. Consider:
>
> [gstein@kurgan mod_dav_svn]$ svn update . ~/src/svn/notes
>
>
> (2) is no big deal. The WC has functions to detect if a given dir is a WC
> dir or not. Note that a basepath outside of a WC usually means you have
> disjoint WC's (and possibly different repositories).
>
> (3) shouldn't be a problem. That is easily discoverable.
>
> These last two points should be noted in the doc, so that callers can
> understand the issues.
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
Received on Sat Oct 21 14:36:21 2006