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

Re: svn commit: rev 1581 - trunk/subversion/include trunk/subversion/libsvn_wc trunk/subversion/libsvn_client

From: <cmpilato_at_collab.net>
Date: 2002-03-27 00:15:20 CET

Greg Stein <gstein@lyra.org> writes:

> On Thu, Mar 21, 2002 at 08:09:00PM -0600, cmpilato@tigris.org wrote:
> >...
> > +++ trunk/subversion/libsvn_client/commit_util.c Thu Mar 21 20:09:00 2002
> >...
> > @@ -714,13 +751,34 @@
> > svn_path_split (item_url, &item_dir, &item_name, pool);
> > if (item_dir->len > common->len)
> > {
> > - int j, count;
> > + char *rel = apr_pstrdup (pool, item_dir->data);
> > + char *piece = rel + common->len + 1;
> >
> > - count = count_components (item_dir->data + common->len + 1);
> > - for (j = 0; j < count; j++)
> > + while (1)
> > {
> > - SVN_ERR (push_stack ("foo", db_stack, &stack_ptr,
> > - editor, NULL, j, FALSE, pool));
> > + /* Find the first separator. */
> > + piece = strchr (piece, '/');
>
> Could this whole loop be replaced with svn_path_decompose() ?

I was going to use that, but decided not to bother with the overhead
of creating an array and duping each component into the array and ...

> > +static void *
> > +make_baton (const char *path, apr_pool_t *pool)
> > +{
> > + struct edit_baton *new_baton
> > + = apr_pcalloc (pool, sizeof (struct edit_baton *));
>
> Might want to use 'sizeof(*new_baton)'

Ah, yes.

> > + new_baton->path = apr_pstrdup (pool, path);
> > + return ((void *) new_baton);
>
> That cast isn't needed. The cast to 'void *' is automatic.

Gotcha.

> >...
> > @@ -754,8 +823,8 @@
> > void **root_baton)
> > {
> > struct edit_baton *eb = edit_baton;
> > - printf ("TEST EDIT STARTED (base url=%s)\n", eb->base_url);
> > - *root_baton = edit_baton;
> > + printf ("TEST EDIT STARTED (base url=%s)\n", eb->path);
> > + *root_baton = make_baton (eb->path, dir_pool);
>
> Since the path can't be changed in the baton (it's const), then can't you
> just use the edit_baton for the root baton?
>
> (of course, the make_baton() will be needed if you're going to put something
> different [from edit_baton] into dir_baton)

It's a temporary editor which will disappear once the testing is
complete. I simply don't care about nits of this magnitude. :-)

> >...
> > +close_item (void *baton)
> > +{
> > + struct edit_baton *this = baton;
>
> We might want to avoid variable names that are C++ keywords.

Gah! You're right.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 27 00:15:31 2002

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.