fitz@tigris.org writes:
> Author: fitz
> Date: Thu Mar 10 15:52:46 2005
> New Revision: 13359
> --- branches/locking/subversion/include/svn_error.h (original)
> +++ branches/locking/subversion/include/svn_error.h Thu Mar 10 15:52:46 2005
> @@ -218,6 +218,21 @@
>
> /** @} */
>
> +/*
> + * @since New in 1.2.
> + *
> + * Return TRUE if @a err is an error specifically related to locking a
> + * path, FALSE otherwise. */
> +svn_boolean_t svn_error_is_lock_error (svn_error_t *err);
The implementation allows err to be NULL, but the documentation
doesn't mention it which means callers cannot rely on it.
> +
> +/*
> + * @since New in 1.2.
> + *
> + * Return TRUE if @a err is an error specifically related to unlocking
> + * a path, FALSE otherwise. */
> +svn_boolean_t svn_error_is_unlock_error (svn_error_t *err);
ditto
> +
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
>
> --- branches/locking/subversion/libsvn_client/locking_commands.c (original)
> +++ branches/locking/subversion/libsvn_client/locking_commands.c Thu Mar 10 15:52:46 2005
> @@ -104,18 +104,24 @@
> struct lock_baton *lb = baton;
> svn_wc_adm_access_t *adm_access;
> const char *abs_path;
> -
> +
> abs_path = svn_path_join (svn_wc_adm_access_path (lb->adm_access),
> path, lb->pool);
> -
> +
> SVN_ERR (svn_wc_adm_probe_retrieve (&adm_access, lb->adm_access,
> abs_path, lb->pool));
> -
> +
> if (do_lock)
> - SVN_ERR (svn_wc_add_lock (abs_path, lock, adm_access, lb->pool));
> - else
> - SVN_ERR (svn_wc_remove_lock (abs_path, adm_access, lb->pool));
> + if (!ra_err)
> + SVN_ERR (svn_wc_add_lock (abs_path, lock, adm_access, lb->pool));
>
> + if (!do_lock)
> + /* Remove our wc lock token either a) if we got no error, or b) if
> + we got any locking-related error except for owner mismatch. */
> + if (!ra_err ||
> + (ra_err && (ra_err->apr_err != SVN_ERR_FS_LOCK_OWNER_MISMATCH)))
> + SVN_ERR (svn_wc_remove_lock (abs_path, adm_access, lb->pool));
The comment reads "any locking-related error except for owner mismatch"
but the code appears to implement "any error except for owner mismatch".
Now I suppose you might have meant that all the ra_err errors are
"locking-related", but perhaps you missed out some code?
I'd prefer it if the comment and code were a more obvious match,
that's assuming that the comment really is adding value; merely
duplicating the code doesn't add much.
> --- branches/locking/subversion/libsvn_ra_dav/session.c (original)
> +++ branches/locking/subversion/libsvn_ra_dav/session.c Thu Mar 10 15:52:46 2005
> @@ -1104,17 +1104,28 @@
> apr_ssize_t keylen;
> void *val;
> svn_revnum_t *revnum;
> + svn_error_t *err, *callback_err;
>
> apr_hash_this(hi, &key, &keylen, &val);
> path = key;
> revnum = val;
>
> - SVN_ERR (shim_svn_ra_dav__lock(session, &lock, path, comment,
> - force, *revnum, pool));
> + err = shim_svn_ra_dav__lock(session, &lock, path, comment,
> + force, *revnum, pool);
> +
> + if (err && !svn_error_is_lock_error (err))
The implementation checks for a NULL err (although you can't rely on
that unless the documentation changes).
> + return err;
>
> /* Run the lock callback if we have one. */
> if (lock_func)
> - SVN_ERR(lock_func(lock_baton, path, TRUE, lock, NULL));
> + callback_err = lock_func(lock_baton, path, TRUE, lock, err);
> +
> + /* clear the error if there was one. */
That comment is pointless, it adds nothing.
> + if (err)
> + svn_error_clear (err);
svn_error_clear checks for NULL so the caller doesn't have to.
> --- branches/locking/subversion/libsvn_subr/error.c (original)
> +++ branches/locking/subversion/libsvn_subr/error.c Thu Mar 10 15:52:46 2005
> @@ -393,3 +393,27 @@
>
> return apr_strerror (statcode, buf, bufsize);
> }
> +
> +svn_boolean_t
> +svn_error_is_lock_error (svn_error_t *err)
> +{
> + if (err &&
There is no point handling NULL inputs if you don't document it.
> + (err->apr_err == SVN_ERR_FS_PATH_LOCKED ||
> + err->apr_err == SVN_ERR_FS_OUT_OF_DATE))
> + return TRUE;
> + return FALSE;
> +}
> +
> +svn_boolean_t
> +svn_error_is_unlock_error (svn_error_t *err)
> +{
> + if (err &&
ditto
> + (err->apr_err == SVN_ERR_FS_PATH_NOT_LOCKED ||
> + err->apr_err == SVN_ERR_FS_BAD_LOCK_TOKEN ||
> + err->apr_err == SVN_ERR_FS_LOCK_OWNER_MISMATCH ||
> + err->apr_err == SVN_ERR_FS_NO_SUCH_LOCK ||
> + err->apr_err == SVN_ERR_RA_NOT_LOCKED ||
> + err->apr_err == SVN_ERR_FS_LOCK_EXPIRED))
> + return TRUE;
> + return FALSE;
> +}
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 11 01:04:19 2005