[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: review of svn_wc_entries_read()

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 19 Mar 2009 01:47:35 +0100

update_editor.c::complete_directory() is another function that fetch
entry structures from the hash and modifies them in-place. Thankfully,
it follows these modifications by a call to svn_wc__entries_write().

Nevertheless, it is a function which needs to be reviewed and
documented. It *seems* safe, but you should review in detail. There is
a call to svn_wc__entry_remove() in there which certainly complicates
things.

Also:
* crop.c::crop_children(). see "dot_entry"
* crop.c::svn_wc_crop_tree(). see "target_entry"
* copy.c::post_copy_cleanup()

The above list is not exhaustive. Just what I've been able to find so far.

I'm about to commit some changes around entry handling that tightens
scopes, adds const, etc, in order to make it clearer where changes are
made to entries. Just running the test suite now.

Cheers,
-g

On Thu, Mar 19, 2009 at 00:16, Greg Stein <gstein_at_gmail.com> wrote:
> Hey Hyrum,
>
> Per our IRC conversation, I'm concerned about removing the internal
> caching of the entries hash. Some functions, contrary to the docstring
> of svn_wc_entries_read(), modify the returned hash with an expectation
> that the changes will "stick" until something syncs/writes those
> changes to disk. In particular svn_wc__entries_modify() is one of
> those functions. Before any further removal of the caching can be
> done, the callers who expect this modify-in-place behavior will need
> to be rewritten.
>
> I found there are 5 callers of svn_wc_entries_read() in libsvn_client,
> and 42 callers in libsvn_wc.
>
> Examining libsvn_client, I see the following breakdown:
>
> commit.c: wants count of versioned children
> commit_util.c: uses values as const
> export.c: uses the keys (names) and entry->kind
> merge.c: two uses that only use the hash keys (ie. they want the names
> of the children)
>
> In short, all of these uses are "safe" and within the confines of the
> intended usage.
>
> For future reference: it seems that libsvn_client would want a
> function to return a list of the children's names (hitting 3 of the 5
> uses). export.c wants the kind, and commit_util needs "everything".
> Note that I'm not examining svn_wc_entry() calls her. That returns a
> "const" value, so I doubt anybody is attempting to modify the entry
> in-place.
>
> I have not (yet) examined the libsvn_wc uses. If anybody will "play
> games", then it will be in there, where there is some level of
> knowledge of the internal workings of the library.
>
> Cheers,
> -g
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1351933
Received on 2009-03-19 01:47:51 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.