On 20.03.2017 17:22, Bert Huijben wrote:
-----Original Message-----
From: stef...@apache.org [mailto:stef...@apache.org]
Sent: vrijdag 10 maart 2017 21:57
To: comm...@subversion.apache.org
Subject: svn commit: r1786446 -
/subversion/trunk/subversion/libsvn_fs_fs/transaction.c
Author: stefan2
Date: Fri Mar 10 20:56:59 2017
New Revision: 1786446
URL: http://svn.apache.org/viewvc?rev=1786446&view=rev
Log:
* subversion/libsvn_fs_fs/transaction.c
(get_shared_rep): Don't leak error objects.
Found by: danielsh
Modified:
subversion/trunk/subversion/libsvn_fs_fs/transaction.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tra
nsaction.c?rev=1786446&r1=1786445&r2=1786446&view=diff
==========================================================
====================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Fri Mar 10
20:56:59 2017
@@ -2429,9 +2429,6 @@ get_shared_rep(representation_t **old_re
err = svn_stream_contents_same2(&same, contents, old_contents,
scratch_pool);
- /* Restore FILE's read / write position. */
- SVN_ERR(svn_io_file_seek(file, APR_SET, &old_position, scratch_pool));
-
/* A mismatch should be extremely rare.
* If it does happen, log the event and don't share the rep. */
if (!same || err)
@@ -2462,6 +2459,9 @@ get_shared_rep(representation_t **old_re
svn_error_clear(err);
*old_rep = NULL;
}
+
+ /* Restore FILE's read / write position. */
+ SVN_ERR(svn_io_file_seek(file, APR_SET, &old_position, scratch_pool));
I'm not sure if that is a safe fix... Need more context for that.
You could just have used svn_error_compose_create() and combine both errors.
Thanks for the review!
I think the code is safe as it stands: err will be chained
by svn_error_createf and then gets cleaned up.
Is there something I'm missing?
-- Stefan^2.
