C. Michael Pilato wrote on Tue, Mar 15, 2011 at 09:05:56 -0400:
> On 03/15/2011 12:34 AM, Daniel Shahaf wrote:
> > cmpilato_at_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);
Received on 2011-03-15 14:16:17 CET