Another bit of trivia regarding error handling: does this look OK?
- Julian
Don't gratuitously hide errors. Tidy error handling code. Fix error leaks
in test programs, not that leaks really matter there but in one case it
simplifies the code and in the other cases it provides extra information.
* subversion/libsvn_wc/copy.c
(svn_wc__remove_wcprops): Eliminate a temporary variable.
* subversion/libsvn_wc/lock.c
(do_open, svn_wc_adm_retrieve): Don't hide the underlying error, attach it
to the chain so it will be seen.
* subversion/libsvn_wc/update_editor.c
(leftmod_error_chain): Remove a redundant initialisation and eliminate
a temporary variable.
(apply_textdelta): Eliminate a temporary variable.
* subversion/tests/libsvn_fs_base/fs-base-test.c
(abort_txn, delete_mutables): Don't leak an error, attach it to the chain.
* subversion/tests/libsvn_fs_base/strings-reps-test.c
(test_strings, abort_string): Don't leak an error, attach it to the chain.
* subversion/tests/libsvn_wc/translate-test.c
(substitute_and_verify): Rather than formatting an error as text (and
leaking it), attach it to the chain; this is simpler and more accurate.
Index: subversion/libsvn_wc/copy.c
===================================================================
--- subversion/libsvn_wc/copy.c (revision 18347)
+++ subversion/libsvn_wc/copy.c (working copy)
@@ -48,7 +48,6 @@
apr_hash_index_t *hi;
const char *wcprop_path;
apr_pool_t *subpool = svn_pool_create (pool);
- svn_error_t *err;
/* Read PATH's entries. */
SVN_ERR (svn_wc_entries_read (&entries, adm_access, FALSE, subpool));
@@ -57,9 +56,7 @@
SVN_ERR (svn_wc__wcprop_path (&wcprop_path,
svn_wc_adm_access_path (adm_access),
svn_node_dir, FALSE, subpool));
- err = svn_io_remove_file (wcprop_path, subpool);
- if (err)
- svn_error_clear (err);
+ svn_error_clear (svn_io_remove_file (wcprop_path, subpool));
/* Recursively loop over all children. */
for (hi = apr_hash_first (subpool, entries); hi; hi = apr_hash_next (hi))
@@ -86,9 +83,7 @@
{
SVN_ERR (svn_wc__wcprop_path (&wcprop_path, child_path,
svn_node_file, FALSE, subpool));
- err = svn_io_remove_file (wcprop_path, subpool);
- if (err)
- svn_error_clear (err);
+ svn_error_clear (svn_io_remove_file (wcprop_path, subpool));
/* ignoring any error value from the removal; most likely,
apr_file_remove will complain about trying to a remove a
file that's not there. But this more efficient than
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c (revision 18347)
+++ subversion/libsvn_wc/lock.c (working copy)
@@ -457,9 +457,7 @@
pool);
if (err)
{
- /* Should we attempt to distinguish certain errors? */
- svn_error_clear (err);
- return svn_error_createf (SVN_ERR_WC_NOT_DIRECTORY, NULL,
+ return svn_error_createf (SVN_ERR_WC_NOT_DIRECTORY, err,
_("'%s' is not a working copy"),
svn_path_local_style (path, pool));
}
@@ -784,8 +782,7 @@
message. */
if (err)
{
- svn_error_clear (err);
- return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
+ return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, err,
_("Unable to check path existence for '%s'"),
svn_path_local_style (path, pool));
}
@@ -815,8 +812,7 @@
message. */
if (err)
{
- svn_error_clear (err);
- return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
+ return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, err,
_("Unable to check path existence for '%s'"),
svn_path_local_style (wcpath, pool));
}
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 18347)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -865,7 +865,7 @@
const char *path,
apr_pool_t *pool)
{
- svn_error_t *tmp_err = SVN_NO_ERROR;
+ svn_error_t *tmp_err;
if (! err)
return SVN_NO_ERROR;
@@ -874,18 +874,14 @@
a local mod was left, or to the NULL end of the chain. */
for (tmp_err = err; tmp_err; tmp_err = tmp_err->child)
if (tmp_err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD)
- {
- break;
- }
-
+ break;
+
/* If we found a "left a local mod" error, wrap and return it.
Otherwise, we just return our top-most error. */
if (tmp_err)
{
/* Remove the LOGFILE (and eat up errors from this process). */
- svn_error_t *err2 = svn_io_remove_file (logfile, pool);
- if (err2)
- svn_error_clear (err2);
+ svn_error_clear (svn_io_remove_file (logfile, pool));
return svn_error_createf
(SVN_ERR_WC_OBSTRUCTED_UPDATE, tmp_err,
@@ -1620,13 +1616,8 @@
if (err && !APR_STATUS_IS_ENOENT(err->apr_err))
{
if (hb->source)
- {
- svn_error_t *err2 = svn_wc__close_text_base (hb->source,
- fb->path,
- 0, handler_pool);
- if (err2)
- svn_error_clear (err2);
- }
+ svn_error_clear (svn_wc__close_text_base (hb->source, fb->path,
+ 0, handler_pool));
svn_pool_destroy (handler_pool);
return err;
}
Index: subversion/tests/libsvn_fs_base/strings-reps-test.c
===================================================================
--- subversion/tests/libsvn_fs_base/strings-reps-test.c (revision 18347)
+++ subversion/tests/libsvn_fs_base/strings-reps-test.c (working copy)
@@ -621,7 +621,7 @@
return svn_error_create (SVN_ERR_FS_GENERAL, NULL,
"query unexpectedly successful");
if (err->apr_err != SVN_ERR_FS_NO_SUCH_STRING)
- return svn_error_create (SVN_ERR_FS_GENERAL, NULL,
+ return svn_error_create (SVN_ERR_FS_GENERAL, err,
"query failed with unexpected error");
svn_error_clear (err);
}
@@ -715,7 +715,7 @@
err = svn_fs_base__retry_txn (args.fs, txn_body_string_append_fail,
&args2, pool);
if ((! err) || (err->apr_err != SVN_ERR_TEST_FAILED))
- return svn_error_create (SVN_ERR_TEST_FAILED, NULL,
+ return svn_error_create (SVN_ERR_TEST_FAILED, err,
"failed to intentionally abort a trail");
svn_error_clear (err);
}
Index: subversion/tests/libsvn_fs_base/fs-base-test.c
===================================================================
--- subversion/tests/libsvn_fs_base/fs-base-test.c (revision 18347)
+++ subversion/tests/libsvn_fs_base/fs-base-test.c (working copy)
@@ -435,7 +435,7 @@
if (err && (err->apr_err != SVN_ERR_FS_NO_SUCH_TRANSACTION))
{
return svn_error_create
- (SVN_ERR_FS_GENERAL, NULL,
+ (SVN_ERR_FS_GENERAL, err,
"opening non-existent txn got wrong error");
}
else if (! err)
@@ -649,7 +649,7 @@
if (err && (err->apr_err != SVN_ERR_FS_ROOT_DIR))
{
return svn_error_createf
- (SVN_ERR_FS_GENERAL, NULL,
+ (SVN_ERR_FS_GENERAL, err,
"deleting root directory got wrong error");
}
else if (! err)
Index: subversion/tests/libsvn_wc/translate-test.c
===================================================================
--- subversion/tests/libsvn_wc/translate-test.c (revision 18347)
+++ subversion/tests/libsvn_wc/translate-test.c (working copy)
@@ -329,14 +329,10 @@
}
else if (err->apr_err != SVN_ERR_IO_INCONSISTENT_EOL)
{
- char buf[1024];
-
- svn_strerror (err->apr_err, buf, sizeof (buf));
-
return svn_error_createf
- (SVN_ERR_TEST_FAILED, NULL,
- "translation of '%s' should fail, but not with error \"%s\"",
- src_fname, buf);
+ (SVN_ERR_TEST_FAILED, err,
+ "translation of '%s' should fail, but not with this error",
+ src_fname);
}
else
{
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Feb 6 21:08:29 2006