On Thu, 07 Dec 2006, Madan S. wrote:
> On Tue, 05 Dec 2006 23:45:32 +0530, Daniel Rall <dlr@collab.net> wrote:
>
> >On Tue, 05 Dec 2006, Madan S. wrote:
> >
> >>On Tue, 05 Dec 2006 00:42:39 +0530, Daniel Rall <dlr@collab.net> wrote:
...
> >Yes, the name doesn't match exactly what we want to do, as we're not
> >actually changing any WC properties here. Too bad the field isn't
> >named prop_changes. Hmmmmmm.
> >
> >All our options here appear to involving rev'ing the data structure
> >and its callers. :-\
> >
> >a) Rename wcprop_changes to prop_changes, and use it for both extra
> >client -> repos and repos -> WC property edits, with an API contract
> >indicating that it must be cleared after the client -> repos prop
> >edits. This could be confusing to developers using the structure for
> >the first time, but might use less memory.
> >
> >b) Add a repos_prop_changes field for client -> repos prop edits.
> >
> >Either way, wcprop_changes could REALLY use some better documentation!
>
> I would opt for (a) which would mean one more commit (to rename and
> change all occurances of the old name) before we can check this into
> trunk.
Why do you think option (a) is better? I was leaning towards (b) on
the grounds that it could provide a more obvious API, but I could
really go either way.
If we go with option (a), a single commit is actually better, since we
have to rev API, since we're changing a field name in the the public
the commit item data structure.
> >>But I guess like you said above, setting restting the wcprop_changes
> >>with a new array (not just setting nelts to 0) would not interfere with
> >>the current implementation.
...
> >The doc string on apr_array_header_t's nelts field says "The number of
> >active elements in the array". It has a separate field, nalloc, which
> >represents "The number of elements allocated in the array". My
> >understanding of apr_array_header_t is that manipulating nelts is an
> >acceptable way to shrink or clear the list (though there's also the
> >apr_array_pop() API).
> >
> >process_committed_leaf() makes proper use of the nelts field:
> >
> > /* Do wcprops in the same log txn as revision, etc. */
> > if (wcprop_changes && (wcprop_changes->nelts > 0))
> > {
>
> True, but this sanity should not be expected by default. Am not an expert
> in APR, so you are most probably right... but I would prefer to err on the
> right side :)
...
I've confirmed this with APR's core developers, and submitted a patch
to APR adding an apr_array_clear() API which does exactly this (sets
nelts to 0). We can depend on this behavior.
- application/pgp-signature attachment: stored
Received on Fri Dec 8 02:00:59 2006