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

(take 2) Re: svn commit: rev 5928 - in trunk/subversion: include libsvn_wc

From: Greg Stein <gstein_at_lyra.org>
Date: 2003-05-14 01:53:26 CEST

[ looking at some more context... ]

On Tue, May 13, 2003 at 04:59:08PM -0500, sussman@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_wc/update_editor.c Tue May 13 16:59:06 2003
>...
> @@ -618,7 +586,31 @@
>
> *dir_baton = d = make_dir_baton (NULL, eb, NULL, eb->is_checkout, pool);
> if (eb->is_checkout)
> - SVN_ERR (prep_directory (d, eb->ancestor_url, eb->target_revision, pool));
> + {
> + SVN_ERR (prep_directory (d, eb->ancestor_url,
> + eb->target_revision, pool));
> + }
> + else if (! eb->target)
> + {
> + /* For an update with a NULL target, this is equivalent to open_dir(): */

Then these two blocks of code should be factored out.

> + svn_wc_adm_access_t *adm_access;
> + svn_wc_entry_t tmp_entry;
> + apr_uint32_t modify_flags = 0;

And after noting the above refactor, I'll also note that my previous comment
about 'modify_flags' applies here, too.

>...
> @@ -763,6 +756,16 @@
> db->pool);
> copyfrom_revision = pb->edit_baton->target_revision;
>
> + /* Immediately create an entry for the new directory in the parent.
> + Note that the parent must already be either added or opened, and
> + thus it's in an 'incomplete' state just like the new dir. */
> + tmp_entry.kind = svn_node_dir;
> + tmp_entry.deleted = FALSE;
> + SVN_ERR (svn_wc__entry_modify (adm_access, db->name, &tmp_entry,
> + (SVN_WC__ENTRY_MODIFY_KIND
> + | SVN_WC__ENTRY_MODIFY_DELETED),
> + TRUE /* immediate write */, pool));
> +
> /* Extra check: a directory by this name may not exist, but there
> may still be one scheduled for addition. That's a genuine
> tree-conflict. */

The check that follows this comment fetches the entry for db->name.
Specifically, it has a test for whether the entry exists. Given the above,
it definitely will exist. But more importantly, these two entry
manipulations should be folded together.

Specifically, the __entry_modify() is going to read the entries, but then it
will toss them out. But the following code will read the entries *again*.

Hmm. Further: the following check is to look for an obstructed update. I
would think that should happen *before* you go and create a new entry.
Especially because you might have a file sitting there and the above
modification would force that entry to a directory(!).

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed May 14 01:51:49 2003

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.