PATCH: Errors that are ignored but should be cleared

Is this a correct catch of some more potential memory leaks?

[[[
Clear svn_error_t errors rather than just ignoring them, else there is
a memory leak.
]]]

I also found the following, but we might as well evaluate the "### ... todo" comments and do the right thing while we are thinking about it, so what is the right thing? Just clear and ignore the error, or not?

~/src/subversion> grep -C3 "svn_repos_abort_report" subversion/mod_dav_svn/update.c
            if (! SVN_IS_VALID_REVNUM (rev))
              {
                /* ### This removes the fs txn. todo: check error. */
                svn_repos_abort_report(rbaton);
                serr = svn_error_create (SVN_ERR_XML_ATTRIB_NOT_FOUND,
                                         NULL, "rev");
                return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,

--
            if (serr != NULL)
              {
                /* ### This removes the fs txn.  todo: check error. */
                svn_repos_abort_report(rbaton);
                return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                           "A failure occurred while "
                                           "recording one of the items of "
--
            if (serr != NULL)
              {
                /* ### This removes the fs txn.  todo: check error. */
                svn_repos_abort_report(rbaton);
                return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                           "A failure occurred while "
                                           "recording one of the (missing) "
--
  if (serr != NULL)
    {
      /* ### This removes the fs txn.  todo: check error. */
      svn_repos_abort_report(rbaton);
      return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                 "A failure occurred during the completion "
                                 "and response generation for the update "
- Julian

Clear svn_error_t errors rather than just ignoring them, else there is
a memory leak.

Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 7845)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -718,7 +718,7 @@ wc_to_repos_copy (svn_client_commit_info
  cleanup:
   /* Abort the commit if it is still in progress. */
   if (commit_in_progress)
- editor->abort_edit (edit_baton, pool); /* ignore return value */
+ svn_error_clear (editor->abort_edit (edit_baton, pool));
 
   /* It's only a read lock, so unlocking is harmless. */
   unlock_err = svn_wc_adm_close (adm_access);
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 7845)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -662,7 +662,7 @@ svn_client_import (svn_client_commit_inf
   if ((err = import (path, new_entry,
                      editor, edit_baton, nonrecursive, excludes, ctx, pool)))
     {
- editor->abort_edit (edit_baton, pool);
+ svn_error_clear (editor->abort_edit (edit_baton, pool));
       return err;
     }
 
Index: subversion/libsvn_ra_svn/editorp.c
===================================================================
--- subversion/libsvn_ra_svn/editorp.c (revision 7845)
+++ subversion/libsvn_ra_svn/editorp.c (working copy)
@@ -787,7 +787,7 @@ svn_error_t *svn_ra_svn__drive_editorp(s
           if (!state.done)
             {
               /* Abort the edit and use non-blocking I/O to write the error. */
- editor->abort_edit(edit_baton, subpool);
+ svn_error_clear (editor->abort_edit(edit_baton, subpool));
               svn_ra_svn__set_block_handler(conn, blocked_write, &state);
             }
           write_err = svn_ra_svn_write_cmd_failure(conn, subpool, err->child);
Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c (revision 7845)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -712,7 +712,7 @@ svn_subst_copy_and_translate (const char
   const char *dst_tmp;
   svn_stream_t *src_stream, *dst_stream;
   apr_file_t *s = NULL, *d = NULL; /* init to null important for APR */
- svn_error_t *err, *err2;
+ svn_error_t *err;
 
   /* The easy way out: no translation needed, just copy. */
   if (! (eol_str || keywords))
@@ -739,17 +739,15 @@ svn_subst_copy_and_translate (const char
   if (err)
     {
       /* ignore closure errors if we're bailing. */
- svn_stream_close (src_stream);
- svn_stream_close (dst_stream);
+ svn_error_clear (svn_stream_close (src_stream));
+ svn_error_clear (svn_stream_close (dst_stream));
       if (s)
         apr_file_close (s);
       if (d)
- apr_file_close (d);
+ apr_file_close (d);
 
- err2 = svn_io_remove_file (dst_tmp, pool);
- if (err2)
- svn_error_clear (err2);
- return
+ svn_error_clear (svn_io_remove_file (dst_tmp, pool));
+ return
         svn_error_createf (err->apr_err, err,
                            "file translation failed when copying '%s' to '%s'",
                            src, dst);
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 7845)
+++ subversion/svnserve/serve.c (working copy)
@@ -354,7 +354,7 @@ static svn_error_t *accept_report(svn_ra
   else if (rb.err)
     {
       /* Some failure during the reporting or editing operations. */
- editor->abort_edit(edit_baton, pool);
+ svn_error_clear(editor->abort_edit(edit_baton, pool));
       SVN_CMD_ERR(rb.err);
     }
   SVN_ERR(svn_ra_svn_write_cmd_response(conn, pool, ""));

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 26 03:49:54 2003