I fixed all this stuff in r27253, thanks.
On 10/15/07, Eric Gillespie <epg@google.com> wrote:
> sussman@tigris.org writes:
>
> > Author: sussman
> > Date: Sun Oct 14 14:11:29 2007
> > New Revision: 27180
>
> Looks good! A few minor comments:
>
> > * subversion/svn/conflict-callbacks.c
> > (print_conflict_description): remove crufty old debugging function.
> > (svn_cl__ignore_conflicts): update function signature and use of API.
>
> svn_cl__ignore_conflicts is never used; is it also crufty old?
>
> > +++ trunk/subversion/libsvn_wc/merge.c Sun Oct 14 14:11:29 2007
> > @@ -419,13 +419,23 @@
> > cdesc.their_file = right;
> > cdesc.my_file = tmp_target;
> > cdesc.merged_file = result_target;
> > + cdesc.property_name = NULL;
> >
> > SVN_ERR(conflict_func(&result, &cdesc, conflict_baton, pool));
> > - switch (result)
> > + if (result == NULL)
> > + {
> > + /* If we were harsh, we'd throw an error here.
> > + Instead, we'll just pretend they want to postpone
> > + the conflict resolution. */
> > + result = svn_wc_create_conflict_result(
> > + svn_wc_conflict_choose_postpone, NULL, pool);
> > + }
>
> Ugh, either raise the error or document this behavior in the doc
> string. I vote for the former. Again later in this function, in
> the binary-handling version.
>
> > @@ -684,13 +706,33 @@
> > user's file, we do nothing. We also do nothing if
> > the response claims to have already resolved the
> > problem.*/
> > - case svn_wc_conflict_result_resolved:
> > - case svn_wc_conflict_result_choose_mine:
> > + case svn_wc_conflict_choose_mine:
> > {
> > *merge_outcome = svn_wc_merge_merged;
> > contains_conflicts = FALSE;
> > goto merge_complete;
> > }
> > + case svn_wc_conflict_choose_merged:
> > + {
> > + if (! result->merged_file)
> > + {
> > + /* ?? Callback asked us to choose its own
> > + merged file, but didn't provide one? */
> > + contains_conflicts = TRUE;
>
> Hmm. I'm inclined to call this an API violation as well, and
> raise an error.
>
> > +svn_wc_conflict_result_t *
> > +svn_wc_create_conflict_result(svn_wc_conflict_choice_t choice,
> > + const char *merged_file,
> > + apr_pool_t *pool)
> > +{
> > + svn_wc_conflict_result_t *result = apr_pcalloc(pool, sizeof(*result));
>
> You explicitly initialize the only two fields, so just use palloc
> instead of pcalloc.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Oct 17 20:13:33 2007