On 1/2/11 12:06 PM, Daniel Shahaf wrote:
Blair Zajac wrote on Sun, Jan 02, 2011 at 11:51:45 -0800:
On 1/1/2011 11:19 AM, danie...@apache.org wrote:
Author: danielsh
Date: Sat Jan 1 19:19:53 2011
New Revision: 1054273
URL: http://svn.apache.org/viewvc?rev=1054273&view=rev
Log:
Avoid hard-coding a line number in a C test.
* subversion/include/svn_error_codes.h
(SVN_ERR_ASSERTION_ONLY_TRACING_LINKS): New error code.
* subversion/libsvn_subr/error.c
(svn_error_purge_tracing):
Return a specific error code, rather than a specific line number :-).
* subversion/tests/libsvn_subr/error-test.c
(test_error_purge_tracing):
Update expectations. While here, fix a bug where ERR3_COPY would
potentially be uninitialized.
There isn't a bug here. err3_copy would only be used if err3 is not
NULL, which is enforced by the SVN_TEST_ASSERT(err3) before err3_copy is
used. So this code can be removed from the test:
else
err3_copy.apr_err = APR_SUCCESS;
Ah, I see. You are correct, of course, the standing code was correct.
But, I think it's better not to rely on that; someone could refactor the
code tomorrow and leave err3_copy uninitialized.
The code doesn't make any assumptions about the return from
svn_error_purge_tracing() and as it asserts various conditions (such as the
input error and output error having different pools), and as it discovers what
is returned, it than can clear the errors. The errors are cleared as soon as it
is safe, so I don't think the ordering can change much.
Could we just move the SVN_TEST_ASSERT(err3) further up, and remove the
"if (err3)" entirely?
If the SVN_TEST_ASSERT(err3) is moved up, then it will happen before
svn_error_clear(err), which if the assertion fails, leads to an error leak.
Blair
