Re: Bug in copy-cmd.c?

Stefan, can you please repost this -- with context from Barry's mail if
necessary -- as a new thread with the [PATCH] Subject: line prefix? That
way our patch manager has a better chance of seeing it.

Stefan Sperling wrote:
> 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;
> }
>

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand