"C. Michael Pilato" <cmpil...@collab.net> writes: > [Tweaking Subject: for (hopefully) additional visibility.] > > On 03/15/2011 09:15 AM, Daniel Shahaf wrote: >> 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);
It has a different problem, if the client modifies the context in a callback those modifications will not persist. -- Philip
