VK Sameer wrote:
> On Tue, 2005-07-05 at 21:07 +0100, Julian Foad wrote:
>>>VK Sameer <sameer@collab.net> writes:
>>>>+ err = handle_auth_request(sess, pool);
>>>>+
>>>>+ /* Pre-1.3 servers don't support 'lock-many'. If that fails, fall back
>>>>+ * to 'lock'. */
>>>>+ if (err && err->apr_err == SVN_ERR_RA_SVN_UNKNOWN_CMD &&
>>>>+ strcmp (err->message, "Unknown command 'lock-many'") == 0)
>>>>+ return ra_svn_lock_compat(session, path_revs, comment, force, lock_func,
>>>>+ lock_baton, pool);
>>
>>(I'm chipping in with no knowledge again.) Is it really necessary and
>>appropriate to do a string comparison of the error message text? It appears to
>>me that just testing the error code would be sufficient.
>
> I don't have a strong opinion on this. It is mainly (ultra-?) defensive
> programming for a garbled marshalling/unmarshalling situation. If that
> seems far-fetched, I'll take it out.
That does seem far-fetched.
The way I think about issues like this is to imagine two Subversion systems
side by side, one that includes this test, and the other identical except that
it does not. I ask myself, "Is there any situation in which the former would
behave better than the latter?"
In this case I can't imagine any such situation. If a server responds to
"lock-many" with UNKNOWN_CMD:"Some unexpected text", it must be a communication
error or a broken server or a customised server (e.g. translated into another
language). If the communication or the server is broken, I can't imagine how
attempting a plain "lock" could do any more harm. If it is a customised
server, then we really want to attempt the plain "lock". Therefore I believe
this comparison (and the similar one for "unlock") should not be present.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 6 10:34:20 2005