You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Glasser <gl...@davidglasser.net> on 2008/04/09 17:55:53 UTC
error leak in do_file_merge
In libsvn_client/merge.c(do_file_merge), which is code I don't know
very well, I observe:
if (i < remaining_ranges->nelts - 1 &&
is_path_conflicted_by_merge(merge_b))
{
err = make_merge_conflict_error(target_wcpath, r, pool);
break;
}
}
/* Record updated WC mergeinfo to account for our new merges, minus
any unresolved conflicts and skips. */
if (record_mergeinfo && remaining_ranges->nelts)
{
apr_hash_t *merges;
SVN_ERR(determine_merges_performed(&merges, target_wcpath,
&range, svn_depth_infinity,
adm_access, notify_b, merge_b,
subpool));
/* If merge target has indirect mergeinfo set it before
recording the first merge range. */
if (indirect)
SVN_ERR(svn_client__record_wc_mergeinfo(target_wcpath,
target_mergeinfo,
adm_access, subpool));
SVN_ERR(update_wc_mergeinfo(target_wcpath, entry, mergeinfo_path,
merges, is_rollback, adm_access,
ctx, subpool));
}
svn_pool_destroy(subpool);
/* Sleep to ensure timestamp integrity. */
svn_sleep_for_timestamps();
return err;
}
Hmm, that "if (record_mergeinfo && " block seems suspect to me.
Either (a) it should be skipped if 'err' is set or (b) it needs to
catch errors in the three calls it makes and clear either err or the
new error. I have no idea which one, though.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: error leak in do_file_merge
Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> I don't know this bit of the code that well, but I wonder if something
> like the attached works better.
By the way, I committed this in r30487.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: error leak in do_file_merge
Posted by "C. Michael Pilato" <cm...@collab.net>.
I don't know this bit of the code that well, but I wonder if something like
the attached works better.
David Glasser wrote:
> In libsvn_client/merge.c(do_file_merge), which is code I don't know
> very well, I observe:
>
> if (i < remaining_ranges->nelts - 1 &&
> is_path_conflicted_by_merge(merge_b))
> {
> err = make_merge_conflict_error(target_wcpath, r, pool);
> break;
> }
> }
>
> /* Record updated WC mergeinfo to account for our new merges, minus
> any unresolved conflicts and skips. */
> if (record_mergeinfo && remaining_ranges->nelts)
> {
> apr_hash_t *merges;
> SVN_ERR(determine_merges_performed(&merges, target_wcpath,
> &range, svn_depth_infinity,
> adm_access, notify_b, merge_b,
> subpool));
> /* If merge target has indirect mergeinfo set it before
> recording the first merge range. */
> if (indirect)
> SVN_ERR(svn_client__record_wc_mergeinfo(target_wcpath,
> target_mergeinfo,
> adm_access, subpool));
>
> SVN_ERR(update_wc_mergeinfo(target_wcpath, entry, mergeinfo_path,
> merges, is_rollback, adm_access,
> ctx, subpool));
> }
>
> svn_pool_destroy(subpool);
>
> /* Sleep to ensure timestamp integrity. */
> svn_sleep_for_timestamps();
>
> return err;
> }
>
>
>
> Hmm, that "if (record_mergeinfo && " block seems suspect to me.
> Either (a) it should be skipped if 'err' is set or (b) it needs to
> catch errors in the three calls it makes and clear either err or the
> new error. I have no idea which one, though.
>
> --dave
>
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand