Re: Bug in copy-cmd.c?

On Sun, Nov 04, 2007 at 02:39:23PM +0000, Barry Scott wrote:
> I'm reading svn_cl__copy() in attempt to understand how to call
> svn_client_copy4().
>
> Isn't it a bug that the peg_revision is not allocated from the pool?
> The value of peg_revision is overwritten each time around the loop.

Looks very strange indeed.

You mean this bit, right?

  /* Get the src list and associated peg revs */
  sources = apr_array_make(pool, targets->nelts - 1,
                           sizeof(svn_client_copy_source_t *));
  for (i = 0; i < (targets->nelts - 1); i++)
    {
      const char *target = ((const char **) (targets->elts))[i];
      svn_client_copy_source_t *source = apr_palloc(pool, sizeof(*source));
      const char *src;
      svn_opt_revision_t peg_revision;

      SVN_ERR(svn_opt_parse_path(&peg_revision, &src, target, pool));
      source->path = src;
      source->revision = &(opt_state->start_revision);
      source->peg_revision = &peg_revision;

      APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = source;
    }

As I read it when this loop is done all sources will have the same
peg revision, namely the one of the last source in the list.

This does not look as if it was intended, especially given
that the comment talks about multiple peg revisions.

I think this is the diff Barry has expressed in plain English:

Index: subversion/svn/copy-cmd.c
===================================================================
--- subversion/svn/copy-cmd.c (revision 27585)
+++ subversion/svn/copy-cmd.c (working copy)
@@ -60,12 +60,12 @@
       const char *target = APR_ARRAY_IDX(targets, i, const char *);
       svn_client_copy_source_t *source = apr_palloc(pool, sizeof(*source));
       const char *src;
- svn_opt_revision_t peg_revision;
+ svn_opt_revision_t *peg_revision = apr_palloc(pool, sizeof(*peg_revision));
 
- SVN_ERR(svn_opt_parse_path(&peg_revision, &src, target, pool));
+ SVN_ERR(svn_opt_parse_path(peg_revision, &src, target, pool));
       source->path = src;
       source->revision = &(opt_state->start_revision);
- source->peg_revision = &peg_revision;
+ source->peg_revision = peg_revision;
 
       APR_ARRAY_PUSH(sources, svn_client_copy_source_t *) = source;
     }

-- 
Stefan Sperling <stsp@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner