"VK Sameer" <sameer@collab.net> writes:
> Patch for issue 2264 - svn client and server enhancement to send multiple
> locks over ra_svn. Test #22 in lock_tests.py covers this scenario.
>
> * subversion/include/svn_ra_svn.h
> (svn_ra_svn_handle_failure_status): New function pulled out of
> svn_ra_svn_read_cmd_response to allow lower-level handling of
> cmd response.
> * subversion/libsvn_ra_svn/marshal.c
> (svn_ra_svn_handle_failure_status): New function.
> (svn_ra_svn_read_cmd_response): Refactored to call
> svn_ra_svn_handle_failure_status.
svn_ra_svn_handle_failure_status() is a new, externally public
function, yet its only callers are in libsvn_ra_svn/ (I checked that
in the code as well as here in the log message).
Why isn't it internally (i.e., "svn_ra_svn__handle_failure_status")
instead of externally public?
> * subversion/libsvn_ra_svn/client.c:
> (svn_ra_lock): New function implementing 'lock-many' verb
> (svn_ra_lock0): Old svn_ra_lock function, now the fallback for the
> 'lock' verb.
> (svn_ra_unlock): New function implementing 'unlock-many' verb
> (svn_ra_unlock0): Old svn_ra_unlock function, now the fallback for the
> 'unlock' verb.
You reversed some stuff. Their names are "ra_svn_lock",
"ra_svn_lock0", "ra_svn_unlock", and "ra_svn_unlock0"
Instead of "0", could you use a more descriptive suffix, like
"_single" or something? These are internal functions, there's no need
to reflect our public deprecation conventions in reverse.
> * subversion/libsvn_ra_svn/serve.c:
> (lock_many): New function
> (unlock_many): New function
I haven't looked at the code yet, but I can see in the log message
that you added two static functions to libsvn_ra_svn/serve.c (they
must be static, because they have no "svn_..." prefixes). Since
they're static, they can only be called from within the same file.
Yet there are no other changes listed for that file, so no calls to
these new functions were added. Either:
a) The log message is incomplete, or
b) These functions really are unused :-).
I'm guessing it's (a). On to the code...
> Index: subversion/include/svn_ra_svn.h
> ===================================================================
> --- subversion/include/svn_ra_svn.h (revision 14823)
> +++ subversion/include/svn_ra_svn.h (working copy)
> @@ -267,6 +267,12 @@
> apr_pool_t *pool,
> const char *fmt, ...);
>
> +/** Parse the error messages in @params from a command response with a
> + * failure status and generate errors from its contents using @pool.
> + */
> +svn_error_t *svn_ra_svn_handle_failure_status (apr_pool_t *pool,
> + apr_array_header_t *params);
> +
I realize that there is some extreme (but consistent) terseness going
on already in svn_ra_svn.h, but something about the ordering of the
words here made it difficult to figure out what this does. I had to
read it several times.
The wording above implies that the errors will be allocated in @a pool
(not "@pool", by the way). But they're not; errors are always
allocated in subpools of the global pool. @a pool is only used here
for temporary work.
Also, the function returns exactly one error, yet the doc string
claims it "generates errors". How are these multiple errors returned?
Are they an error chain dangling off one returned error? I guess so,
based on the code later.
Here's an attempt at a restatement:
/** Return an error chain based on @a params (which contains a
* command response indicating failure). The error chain will be
* in the same order as the errors indicated in @a params.
*
* Use @pool for temporary allocation.
*/
Is that right?
> Index: subversion/libsvn_ra_svn/client.c
> ===================================================================
> --- subversion/libsvn_ra_svn/client.c (revision 14823)
> +++ subversion/libsvn_ra_svn/client.c (working copy)
> @@ -1396,7 +1396,7 @@
> return SVN_NO_ERROR;
> }
>
> -static svn_error_t *ra_svn_lock(svn_ra_session_t *session,
> +static svn_error_t *ra_svn_lock0(svn_ra_session_t *session,
> apr_hash_t *path_revs,
> const char *comment,
> svn_boolean_t force,
Whatever suffix you choose, remembre to reindent the other parameters
here too.
> @@ -1410,8 +1410,6 @@
> apr_hash_index_t *hi;
> apr_pool_t *iterpool = svn_pool_create (pool);
>
> - /* ### TODO for 1.3: Send all the locks over the wire at once. This
> - loop is just a temporary shim. */
> for (hi = apr_hash_first(pool, path_revs); hi; hi = apr_hash_next(hi))
> {
> svn_lock_t *lock;
Nice to see *that* comment go away! :-)
> @@ -1459,7 +1457,110 @@
> return SVN_NO_ERROR;
> }
>
> -static svn_error_t *ra_svn_unlock(svn_ra_session_t *session,
> +static svn_error_t *ra_svn_lock(svn_ra_session_t *session,
> + apr_hash_t *path_revs,
> + const char *comment,
> + svn_boolean_t force,
> + svn_ra_lock_callback_t lock_func,
> + void *lock_baton,
> + apr_pool_t *pool)
Heh, that reversal (unlock->lock) is just an artifact of the diffing
algorithm, I guess. Still throws me sometimes, though.
> +{
> + ra_svn_session_baton_t *sess = session->priv;
> + svn_ra_svn_conn_t* conn = sess->conn;
> + apr_array_header_t *list;
> + apr_hash_index_t *hi;
> + int i;
> + svn_ra_svn_item_t *elt;
> + svn_error_t *err, *callback_err = NULL;
> + apr_pool_t *subpool = svn_pool_create (pool);
> + const char *status;
> + apr_array_header_t *save_paths; /* condensed target paths */
> + save_paths = apr_array_make(subpool, 1, sizeof(const char*));
> +
> + /* (lock-many (?c) b ( (c(?r)) ...) ) */
> + // memory leak because subpool is not being destroyed?
> + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "w((?c)b(!",
> + "lock-many", comment, force));
> +
> + for (hi = apr_hash_first(subpool, path_revs); hi; hi = apr_hash_next(hi))
> + {
> + const void *key;
> + const char *path;
> + void *val;
> + svn_revnum_t *revnum;
> +
> + apr_hash_this(hi, &key, NULL, &val);
> + path = key;
> + APR_ARRAY_PUSH(save_paths, const char*) = path;
> + revnum = val;
> +
> + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "c(?r)",
> + path, *revnum));
> + }
Why not use the usual loop pool idiom?
The subpool should be used by the loop, as it already is, but also
cleared at the top of the loop, as described in HACKING, see the
section called "APR pool usage conventions"). The apr_hash_first()
would have to take 'pool', then, of course.
If this were the only loop, the subpool would be destroyed after the
loop. But in this case, you may want to use it again in a later loop
in the same function, so you wouldn't destroy it yet.
> + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "!))"));
Since you already have subpool, you might as well use it here, but
ordinarily you would use 'pool', since this is not in a loop.
However, since there are later loops that will use 'subpool', you
would *definitely* use 'pool' here if you were going to allocate data
that needed to live beyond the next (potential) call to
svn_pool_clear(subpool). Thus, it may be safer to use 'pool' anyway,
because that way this sort of unexpected-clear bug can't be introduced
later.
> + err = handle_auth_request(sess, subpool);
Same.
> + /* 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)
> + {
> + svn_pool_destroy (subpool);
> + return ra_svn_lock0(session, path_revs, comment, force,
> + lock_func, lock_baton, pool);
> + }
No need to destroy the subpool, because you will have been careful to
clear it on each iteration of any loops anyway, so the amount of extra
memory kept if you don't destroy it will be O(1) not O(N).
> + /* Servers before 1.2 doesn't support locking. Check this here. */
> + SVN_ERR(handle_unsupported_cmd(err, _("Server doesn't support the "
> + "lock command")));
> +
> + /* svn_ra_svn_read_cmd_response unusable as it doesn't
> + * return unparsed params. */
> + err = svn_ra_svn_read_tuple(conn, subpool, "wl", &status, &list);
Same about using 'subpool' vs 'pool'.
> + /* Handle failure status explicitly */
> + if (strcmp (status, "failure") == 0)
> + {
> + return svn_ra_svn_handle_failure_status (subpool, list);
> + }
Same.
> + if (err && !SVN_ERR_IS_LOCK_ERROR (err))
> + return err;
> +
> + for (i = 0; i < list->nelts; ++i)
> + {
> + svn_lock_t *lock;
> + const char *saved_path;
> +
> + saved_path = APR_ARRAY_IDX(save_paths, i, const char *);
> + elt = &APR_ARRAY_IDX(list, i, svn_ra_svn_item_t);
> +
> + if (elt->kind != SVN_RA_SVN_LIST)
> + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> + _("Lock element not a list"));
> +
> + err = parse_lock(elt->u.list, subpool, &lock);
> + if (err)
> + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, err,
> + _("unable to parse lock data"));
> +
> + if (lock_func) {
> + callback_err = lock_func(lock_baton, saved_path, TRUE,
> + err ? NULL : lock,
> + err, subpool); }
> +
> + if (callback_err)
> + return callback_err;
> + }
And here's that second loop. Clear 'subpool' at the top of the loop,
as per HACKING.
> + svn_pool_destroy (subpool);
Yes, now you can destroy it.
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *ra_svn_unlock0(svn_ra_session_t *session,
> apr_hash_t *path_tokens,
> svn_boolean_t force,
> svn_ra_lock_callback_t lock_func,
Again, better name.
(If these functions were documented, it would be much easier to know
what's going on here. I realize that this code was not internally
documented when you arrived.)
> @@ -1471,8 +1572,6 @@
> apr_hash_index_t *hi;
> apr_pool_t *iterpool = svn_pool_create (pool);
>
> - /* ### TODO for 1.3: Send all the lock tokens over the wire at once.
> - This loop is just a temporary shim. */
> for (hi = apr_hash_first(pool, path_tokens); hi; hi = apr_hash_next(hi))
> {
> const void *key;
Yay :-).
> @@ -1517,6 +1616,89 @@
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *ra_svn_unlock(svn_ra_session_t *session,
> + apr_hash_t *path_tokens,
> + svn_boolean_t force,
> + svn_ra_lock_callback_t lock_func,
> + void *lock_baton,
> + apr_pool_t *pool)
> +{
> + ra_svn_session_baton_t *sess = session->priv;
> + svn_ra_svn_conn_t* conn = sess->conn;
> + apr_hash_index_t *hi;
> + apr_pool_t *subpool = svn_pool_create (pool);
> + svn_error_t *err, *callback_err;
> +
> + /* (unlock-many (b ( (c(?c)) ...) ) ) */
> + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "w(b(!",
> + "unlock-many",force));
> + for (hi = apr_hash_first(subpool, path_tokens); hi; hi = apr_hash_next(hi))
> + {
> + const void *key;
> + const char *path;
> + void *val;
> + const char *token;
> +
> + apr_hash_this(hi, &key, NULL, &val);
> + path = key;
> +
> + if (strcmp (val, "") != 0)
> + token = val;
> + else
> + token = NULL;
> +
> + SVN_ERR(svn_ra_svn_write_tuple(conn, subpool, "c(?c)",
> + path,token));
> + }
> + SVN_ERR(svn_ra_svn_write_tuple(conn,subpool,"!))"));
> +
> + err = handle_auth_request(sess, subpool);
> +
> + /* Pre-1.3 servers don't support 'unlock_many'. If unknown, fall back
> + * to 'unlock'.
> + */
> + if (err && err->apr_err == SVN_ERR_RA_SVN_UNKNOWN_CMD &&
> + strcmp (err->message, "Unknown command 'unlock-many'") == 0)
> + {
> + svn_error_clear (err);
> + return ra_svn_unlock0(session, path_tokens,
> + force,lock_func,lock_baton,subpool);
> + }
Hey, I have an idea! Since the protocol words are "unlock_many" and
"unlock", why don't you make those be the names of the corresponding
internal functions? In other words, "ra_svn_unlock_many()", etc.
Same goes for the locking functions.
That would make the patch cleaner and easier to review (no old
functions change names), and the code itself would be easier to
follow.
Also, same comments about pool/subpool usage above... and below:
> + /* Servers before 1.2 don't support locking. Check this here. */
> + SVN_ERR(handle_unsupported_cmd(err, _("Server doesn't support the "
> + "unlock command")));
> +
> + /* Unknown error */
> + if (err)
> + return err;
> +
> + err = svn_ra_svn_read_cmd_response(conn, subpool, "");
> +
> + if (err && !SVN_ERR_IS_UNLOCK_ERROR (err))
> + return err;
> +
> + for (hi = apr_hash_first(subpool, path_tokens); hi; hi = apr_hash_next(hi))
> + {
> + const void *key;
> + const char *path;
> +
> + apr_hash_this(hi, &key, NULL, NULL);
> + path = key;
> +
> + if (lock_func)
> + callback_err =
> + lock_func(lock_baton, path, FALSE, NULL, err, subpool);
> + if (callback_err)
> + return callback_err;
> + }
> +
> + svn_error_clear (err);
> + svn_pool_destroy (subpool);
> +
> + return SVN_NO_ERROR;
> +}
...same about pools/subpools.
> @@ -1558,7 +1740,7 @@
>
> /* Servers before 1.2 doesn't support locking. Check this here. */
> SVN_ERR(handle_unsupported_cmd(handle_auth_request(sess, pool),
> - _("Server doesn't support the get-lock "
> + _("Server doesn't support the get-locks "
> "command")));
>
This looks like you added support for handling servers that don't know
about "get-locks", but you *removed* support for handling servers that
don't know about "get-lock". That's bad, and not just because it
makes the above comment inapplicable to the code! :-)
> Index: subversion/libsvn_ra_svn/protocol
> ===================================================================
> --- subversion/libsvn_ra_svn/protocol (revision 14823)
> +++ subversion/libsvn_ra_svn/protocol (working copy)
> @@ -333,10 +333,19 @@
> current-rev:number ] )
> response: ( lock:lockdesc )
>
> + lock-many
> + params: ( [ comment:string ] force:bool ( (path:string [
> + current-rev:number ] ) ... ) )
> + response: ( ( lock:lockdesc ) ... )
> +
> unlock
> params: ( path:string [ token:string ] force:bool )
> response: ( )
>
> + unlock-many
> + params: ( force:bool ( ( path:string [ token:string ] ) ... ) )
> + response: ( )
> +
> get-lock
> params: ( path:string )
> response: ( [ lock:lockdesc ] )
Just a note: I see that "get-locks" is already defined, albeit not in
context here. No problem.
> Index: subversion/libsvn_ra_svn/marshal.c
> ===================================================================
> --- subversion/libsvn_ra_svn/marshal.c (revision 14823)
> +++ subversion/libsvn_ra_svn/marshal.c (working copy)
> @@ -741,17 +741,47 @@
>
> /* --- READING AND WRITING COMMANDS AND RESPONSES --- */
>
> +svn_error_t *svn_ra_svn_handle_failure_status(apr_pool_t *pool,
> + apr_array_header_t *params)
> +{
> + const char *message, *file;
> + svn_error_t *err;
> + svn_ra_svn_item_t *elt;
> + int i;
> + apr_uint64_t apr_err, line;
> +
> + /* Rebuild the error list from the end, to avoid reversing the order. */
> + if (params->nelts == 0)
> + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> + _("Empty error list"));
> + err = NULL;
> + for (i = params->nelts - 1; i >= 0; i--)
> + {
> + elt = &((svn_ra_svn_item_t *) params->elts)[i];
> + if (elt->kind != SVN_RA_SVN_LIST)
> + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> + _("Malformed error list"));
> + SVN_ERR(svn_ra_svn_parse_tuple(elt->u.list, pool, "nccn", &apr_err,
> + &message, &file, &line));
> + /* The message field should have been optional, but we can't
> + easily change that, so "" means a nonexistent message. */
> + if (!*message)
> + message = NULL;
> + err = svn_error_create(apr_err, err, message);
> + err->file = apr_pstrdup(err->pool, file);
> + err->line = line;
> + }
> + return err;
> +}
Since you're looping, build a subpool and use it where you currently
use 'pool', clearing it at the top of each iteration, etc.
(Yes, I realize you just copied this code from elsewhere.)
> svn_error_t *svn_ra_svn_read_cmd_response(svn_ra_svn_conn_t *conn,
> apr_pool_t *pool,
> const char *fmt, ...)
> {
> va_list ap;
> - const char *status, *message, *file;
> + const char *status;
> apr_array_header_t *params;
> svn_error_t *err;
> - svn_ra_svn_item_t *elt;
> - int i;
> - apr_uint64_t apr_err, line;
>
> SVN_ERR(svn_ra_svn_read_tuple(conn, pool, "wl", &status, ¶ms));
> if (strcmp(status, "success") == 0)
> @@ -763,28 +793,7 @@
> }
> else if (strcmp(status, "failure") == 0)
> {
> - /* Rebuild the error list from the end, to avoid reversing the order. */
> - if (params->nelts == 0)
> - return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> - _("Empty error list"));
> - err = NULL;
> - for (i = params->nelts - 1; i >= 0; i--)
> - {
> - elt = &((svn_ra_svn_item_t *) params->elts)[i];
> - if (elt->kind != SVN_RA_SVN_LIST)
> - return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> - _("Malformed error list"));
> - SVN_ERR(svn_ra_svn_parse_tuple(elt->u.list, pool, "nccn", &apr_err,
> - &message, &file, &line));
> - /* The message field should have been optional, but we can't
> - easily change that, so "" means a nonexistent message. */
> - if (!*message)
> - message = NULL;
> - err = svn_error_create(apr_err, err, message);
> - err->file = apr_pstrdup(err->pool, file);
> - err->line = line;
> - }
> - return err;
> + return svn_ra_svn_handle_failure_status (pool, params);
> }
Yeah, that code should have been doing the loop-pool thing, although
in practice I'm sure it never matters, since error chains don't get
that long.
> return svn_error_createf(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> Index: subversion/svnserve/serve.c
> ===================================================================
> --- subversion/svnserve/serve.c (revision 14823)
> +++ subversion/svnserve/serve.c (working copy)
> @@ -1407,6 +1407,78 @@
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *lock_many(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> + apr_array_header_t *params, void *baton)
Misindentation.
> +{
> + apr_array_header_t *locks;
> + apr_array_header_t *lock_cmds;
> + server_baton_t *b = baton;
> + const char *comment;
> + svn_boolean_t force;
> + int i;
> + struct lock_cmd {
> + const char *path;
> + const char *full_path;
> + svn_revnum_t current_rev;
> + svn_lock_t *l;
> + };
> +
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?c)bl", &comment, &force,
> + &locks));
> +
> + lock_cmds =
> + apr_array_make(pool, locks->nelts, sizeof(struct lock_cmd));
> +
> + /* Loop through the lock commands */
> + for (i = 0; i < locks->nelts; ++i)
> + {
> + struct lock_cmd *cmd = apr_pcalloc(pool, sizeof(struct lock_cmd));
> + svn_ra_svn_item_t *item =
> + &APR_ARRAY_IDX(locks, i, svn_ra_svn_item_t);
> +
> + if (item->kind != SVN_RA_SVN_LIST)
> + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> + "Lock commands should be list of lists\n");
> +
> + SVN_ERR(svn_ra_svn_parse_tuple(item->u.list, pool, "c(?r)",
> + &cmd->path, &cmd->current_rev));
> +
> + cmd->full_path = svn_path_join(b->fs_path,
> + svn_path_canonicalize(cmd->path, pool),
> + pool);
> +
> + APR_ARRAY_PUSH(lock_cmds, struct lock_cmd) = *cmd;
> + }
> +
> + SVN_ERR(must_have_write_access(conn, pool, b, TRUE));
> +
> + /* Loop through each path to be locked */
> + for (i = 0; i < lock_cmds->nelts; i++)
> + {
> + struct lock_cmd *cmd =
> + &APR_ARRAY_IDX(lock_cmds, i, struct lock_cmd);
> +
> + SVN_CMD_ERR(
> + svn_repos_fs_lock(&cmd->l, b->repos, cmd->full_path,
> + NULL, comment, 0,
> + 0, /* No expiration time. */
> + cmd->current_rev,
> + force, pool));
> + }
> +
> + /* (success( (ccc(?c)c(?c) ... )) */
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "w(!", "success"));
> + for (i = 0; i < lock_cmds->nelts; i++)
> + {
> + struct lock_cmd *cmd =
> + &APR_ARRAY_IDX(lock_cmds, i, struct lock_cmd);
> + SVN_ERR(write_lock(conn, pool, cmd->l));
> + }
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)"));
> +
> + return SVN_NO_ERROR;
> +}
Again with the loop subpools. But you'll have to be careful, because
some of the data needs to persist beyond the lifetime of the loop in
which it was generated. Choose your pools carefully.
> static svn_error_t *unlock(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> apr_array_header_t *params, void *baton)
> {
> @@ -1419,7 +1491,6 @@
>
> full_path = svn_path_join(b->fs_path, svn_path_canonicalize(path, pool),
> pool);
> -
> /* Username required unless force was specified. */
> SVN_ERR(must_have_write_access(conn, pool, b, ! force));
>
> @@ -1430,6 +1501,61 @@
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *unlock_many(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> + apr_array_header_t *params, void *baton)
Misindentation.
> +{
> + server_baton_t *b = baton;
> + svn_boolean_t force;
> + apr_array_header_t *unlock_tokens;
> + apr_array_header_t *unlock_cmds;
> + struct unlock_cmd {
> + const char *path;
> + const char *token;
> + const char *full_path;
> + };
> + int i;
> +
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "bl",&force, &unlock_tokens));
> +
> + unlock_cmds =
> + apr_array_make(pool, unlock_tokens->nelts, sizeof(struct unlock_cmd));
> +
> + for (i = 0; i< unlock_tokens->nelts; i++)
> + {
> + struct unlock_cmd *cmd = apr_pcalloc(pool,sizeof(struct unlock_cmd));
> + svn_ra_svn_item_t *item = &APR_ARRAY_IDX(unlock_tokens, i,
> + svn_ra_svn_item_t);
> +
> + if (item->kind != SVN_RA_SVN_LIST)
> + return svn_error_create(SVN_ERR_RA_SVN_MALFORMED_DATA, NULL,
> + "Unlock command should be a list of lists\n");
> +
> + SVN_ERR(svn_ra_svn_parse_tuple(item->u.list, pool, "c(?c)",
> + &cmd->path, &cmd->token));
> +
> + cmd->full_path = svn_path_join(b->fs_path,
> + svn_path_canonicalize(cmd->path, pool),pool);
> +
> + APR_ARRAY_PUSH(unlock_cmds, struct unlock_cmd) = *cmd;
> + }
> +
> + /* Username required unless force was specified. */
> + SVN_ERR(must_have_write_access(conn, pool, b, ! force));
> +
> + for (i = 0; i < unlock_tokens->nelts; i++)
> + {
> + struct unlock_cmd *cmd = &APR_ARRAY_IDX(unlock_cmds, i,
> + struct unlock_cmd);
> +
> + SVN_CMD_ERR(svn_repos_fs_unlock(b->repos, cmd->full_path,
> + cmd->token, force, pool));
> + }
> +
> + SVN_ERR(svn_ra_svn_write_cmd_response(conn, pool, ""));
> +
> + return SVN_NO_ERROR;
> +}
Gosh. I feel like there's a big code duplication going on, but I'm
not sure if lock_many and unlock_many can be cleanly factored with
each other. Might be worth trying; your call.
Eagerly awaiting v4,
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 31 00:11:02 2005