C. Michael Pilato wrote on Tue, Mar 15, 2011 at 09:05:56 -0400:
> On 03/15/2011 12:34 AM, Daniel Shahaf wrote:
> > cmpil...@apache.org wrote on Tue, Mar 08, 2011 at 20:05:51 -0000:
> >> Author: cmpilato
> >> Date: Tue Mar  8 20:05:50 2011
> >> New Revision: 1079508
> 
> [...]
> 
> >>  svn_client_commit4(svn_commit_info_t **commit_info_p,
> >>                     const apr_array_header_t *targets,
> 
> [...]
> 
> >> +  /* Ensure that the original notification system is in place. */
> >> +  ctx->notify_func2 = notify_baton.orig_notify_func2;
> >> +  ctx->notify_baton2 = notify_baton.orig_notify_baton2;
> >> +
> > 
> > This is actually a race condition, isn't it? (for clients that call the
> > deprecated API while using CTX->notify_func2 in another thread (in the
> > same or another API))
> > 
> > (Okay, so maybe we'll just let it live on.  Presumably N other
> > deprecated wrappers do this too.)
> 
> I hadn't considered that.  We do alot of pointer swaps like this up in the
> command-line client code itself, but that's a bit different than doing so
> down in the libsvn_client library as in this case.  There is one prior
> instance of us doing this kind of swap inside the libsvn_client library:  in
> libsvn_client/copy.c:repos_to_wc_copy_single().  But I'd prefer mere
> precedent not to be our reason for allowing badness to persist in the 
> codebase.
> 
> Got suggestions?

Would this work? ---

    svn_client_ctx_t ctx2 = *ctx;
    ctx2.notify_func2 = notify_baton.orig_notify_func2;
    ctx2.notify_baton2 = notify_baton.orig_notify_baton2;
    svn_client_commit5(ctx=&ctx2);

Reply via email to