Re: More error leaks

Malcolm Rowe <malcolm-svn-dev@farside.org.uk> writes:

> On Thu, Jan 19, 2006 at 04:10:21PM +0000, Julian Foad wrote:
>> By putting "__attribute__((warn_unused_result))" on each declaration of a
>> function that returns "svn_error_t *", I found the following errors being
>> leaked:
>>
>
> Fantastic job, Julian! I fixed all the trivial instances in r18196.
>
> The ones I didn't fix were:
>
>> subversion/libsvn_diff/diff_file.c: In function
>> 'svn_diff__file_output_unified_default_hdr':
>> subversion/libsvn_diff/diff_file.c:894: return value of 'svn_io_stat'
>
> .. because the function 'can't fail', and:

By "can't fail" do you mean "the calling function doesn't return an
error"? The svn_io_stat call can certainly fail so at the very least
do:

   svn_error_clear (svn_io_stat (...));

or

   err = svn_io_stat (...);
   if (err)
     {
        file_info.mtime = 0;
        svn_error_clear (err);
     }

>> subversion/svn/main.c: In function 'main':
>> subversion/svnadmin/main.c: In function 'main':
>> subversion/svndumpfilter/main.c: In function 'main':
>> subversion/svnlook/main.c: In function 'main':
>> subversion/svnsync/main.c: In function 'main':
>
> .. because the main functions need something slightly more complex
> (a call to svn_cmdline_handle_exit_error(), perhaps).
>
> So those still need some attention.
>
>> Of course the other question raised is: do we want to consider using such
>> attributes as a matter of course? Obviously only if it is done neatly and
>> in a way that doesn't interfere with other compilers; macros can help to
>> achieve that. I'll have a bit more of a think about that.
>>
>
> Yes, we should do this by default, if we can. Any idea how?

No problem, we already use __attribute__ in the public API.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org