You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2012/05/01 00:20:45 UTC

Re: svn commit: r1332432 - /subversion/branches/ev2-export/subversion/libsvn_client/copy.c

On Mon, Apr 30, 2012 at 17:58,  <hw...@apache.org> wrote:
>...
> @@ -902,9 +956,13 @@ repos_to_repos_copy(const apr_array_head
>                                            message, ctx, pool));
>
>   /* Fetch RA commit editor. */
> -  SVN_ERR(svn_ra__register_editor_shim_callbacks(ra_session,
> -                        svn_client__get_shim_callbacks(ctx->wc_ctx,
> -                                                       NULL, pool)));
> +  shim_callbacks = svn_delta_shim_callbacks_default(pool);
> +  SVN_ERR(svn_ra__register_editor_shim_callbacks(ra_session, shim_callbacks));
> +  shim_callbacks->fetch_props_func = fetch_props_func;
> +  shim_callbacks->fetch_base_func = fetch_base_func;
> +  shim_callbacks->fetch_kind_func = fetch_kind_func;
> +  shim_callbacks->fetch_baton = path_infos;

Shouldn't the registration occur *after* the values are assigned? I
recognize that this "works", but it certainly looks strange, and it
would definitely break if the structure is copied into RA-private
storage.

Cheers,
-g

Re: svn commit: r1332432 - /subversion/branches/ev2-export/subversion/libsvn_client/copy.c

Posted by Greg Stein <gs...@gmail.com>.
On Apr 30, 2012 9:26 PM, "Hyrum K Wright" <hy...@wandisco.com> wrote:
>
> On Mon, Apr 30, 2012 at 5:20 PM, Greg Stein <gs...@gmail.com> wrote:
> > On Mon, Apr 30, 2012 at 17:58,  <hw...@apache.org> wrote:
> >>...
> >> @@ -902,9 +956,13 @@ repos_to_repos_copy(const apr_array_head
> >>                                            message, ctx, pool));
> >>
> >>   /* Fetch RA commit editor. */
> >> -  SVN_ERR(svn_ra__register_editor_shim_callbacks(ra_session,
> >> -                        svn_client__get_shim_callbacks(ctx->wc_ctx,
> >> -                                                       NULL, pool)));
> >> +  shim_callbacks = svn_delta_shim_callbacks_default(pool);
> >> +  SVN_ERR(svn_ra__register_editor_shim_callbacks(ra_session,
shim_callbacks));
> >> +  shim_callbacks->fetch_props_func = fetch_props_func;
> >> +  shim_callbacks->fetch_base_func = fetch_base_func;
> >> +  shim_callbacks->fetch_kind_func = fetch_kind_func;
> >> +  shim_callbacks->fetch_baton = path_infos;
> >
> > Shouldn't the registration occur *after* the values are assigned? I
> > recognize that this "works", but it certainly looks strange, and it
> > would definitely break if the structure is copied into RA-private
> > storage.
>
> We can debate the semantics of call-by-value and call-by-reference in
> this instance, but I think your suggestion makes sense: r1332489

Oh, I have no problem with the approach. We have lots of APIs that state
"must live at least as long..." with the implicit by-ref. But we do usually
try to finish initializing the thing before passing a reference :-)

Thanks for the tweak.

Cheers,
-g

Re: svn commit: r1332432 - /subversion/branches/ev2-export/subversion/libsvn_client/copy.c

Posted by Hyrum K Wright <hy...@wandisco.com>.
On Mon, Apr 30, 2012 at 5:20 PM, Greg Stein <gs...@gmail.com> wrote:
> On Mon, Apr 30, 2012 at 17:58,  <hw...@apache.org> wrote:
>>...
>> @@ -902,9 +956,13 @@ repos_to_repos_copy(const apr_array_head
>>                                            message, ctx, pool));
>>
>>   /* Fetch RA commit editor. */
>> -  SVN_ERR(svn_ra__register_editor_shim_callbacks(ra_session,
>> -                        svn_client__get_shim_callbacks(ctx->wc_ctx,
>> -                                                       NULL, pool)));
>> +  shim_callbacks = svn_delta_shim_callbacks_default(pool);
>> +  SVN_ERR(svn_ra__register_editor_shim_callbacks(ra_session, shim_callbacks));
>> +  shim_callbacks->fetch_props_func = fetch_props_func;
>> +  shim_callbacks->fetch_base_func = fetch_base_func;
>> +  shim_callbacks->fetch_kind_func = fetch_kind_func;
>> +  shim_callbacks->fetch_baton = path_infos;
>
> Shouldn't the registration occur *after* the values are assigned? I
> recognize that this "works", but it certainly looks strange, and it
> would definitely break if the structure is copied into RA-private
> storage.

We can debate the semantics of call-by-value and call-by-reference in
this instance, but I think your suggestion makes sense: r1332489

-Hyrum

-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/