makl <makl@tigris.org> writes:
> Philip Martin wrote:
>
>> makl <makl@tigris.org> writes:
>>
>>>Index: subversion/libsvn_wc/lock.c
>>>===================================================================
>>>--- subversion/libsvn_wc/lock.c (revision 9290)
>>>+++ subversion/libsvn_wc/lock.c (working copy)
>>>@@ -329,22 +329,29 @@
>>> static svn_error_t *
>>> do_open (svn_wc_adm_access_t **adm_access,
>>> svn_wc_adm_access_t *associated,
>>>+ const svn_wc_adm_access_t *root_associated,
>> I don't see why you added this additional access baton parameter, it
>> looks like you could just use associated.
>
> Look at the recursion. In the next subdirectory, associated has an empty
> set.
Could the recursive call pass associated rather than the new lock?
>>>+
>>>+ if (lock == &missing)
>>>+ lock = NULL;
>> I think we need to verify the type of the lock, if write_lock is set
>> it's probably not acceptable to use a read-only lock.
>
> missing is a static entry declared in line 80. It has no meaningful
> information, so there is nothing to check.
My comment wasn't directed at missing in particular, but at the fact
that there was no check for the type.
>> Hmm, now I see you are upgrading read-only locks into write locks, but
>> you didn't document this behaviour anywhere. As far as I can tell it
>> even upgrades existing read-only locks! That won't work, not unless
>> the entries cache is thrown away and repopulated. I don't like it,
>> I'd prefer locks not to change type. That's the reason
>> adm_access_alloc takes a lock type; once allocated it never changes.
>
> Can we accept write locks if read locks are requested?
I don't know. If the calling code is making assumptions about the
behaviour of the entries cache (assuming it won't change because it's
read-only) then it's probably safer to return an error.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 8 19:05:10 2004