"C. Michael Pilato" <cmpilato_at_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:
>>>> 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);
It has a different problem, if the client modifies the context in a
callback those modifications will not persist.
--
Philip
Received on 2011-03-15 14:52:57 CET