Re: svn commit: r1054273 - in /subversion/trunk/subversion: include/svn_error.h include/svn_error_codes.h libsvn_subr/error.c tests/libsvn_subr/error-test.c

Blair Zajac wrote on Sun, Jan 02, 2011 at 11:51:45 -0800:
> On 1/1/2011 11:19 AM, danielsh_at_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.

Could we just move the SVN_TEST_ASSERT(err3) further up, and remove the
"if (err3)" entirely?

Daniel
(yes, that's exactly what I suggested last week for err2)

> Blair
Received on 2011-01-02 21:09:21 CET