Re: Warning for missing sentinel arguments

> On 11/18/13 3:03 PM, Julian Foad wrote:

>> The patch also changes SVN_NO_ERROR from "0" to "((svn_error_t *)0)". This
>> has the side effect of detecting other mis-uses: I committed two such fixes
>> as http://svn.apache.org/r1543193 and http://svn.apache.org/r1543216 . I
>> can't think of any negative consequences but shout out if you can.

Actually, this is a change of a public API and maybe ABI (I'm not sure), and while it might be a good idea in itself it should not be casually changed as part of this patch. So I'll leave out that change and not mark svn_cl__try() with SVN_SENTINEL_NULL, since GCC's attribute requires the sentinel argument to be a pointer.

Ben Reser wrote:
> On further thinking I don't think SWIG should be excluded in this case. [...]

Makes sense. Done.

The name 'SVN_SENTINEL_NULL' sounds rather like a value to be used for the sentinel argument itself. I think 'SVN_NEEDS_SENTINEL_NULL' or 'SVN_ATTRIBUTE_SENTINEL_NULL' is better. It doesn't matter that it's longer because it's used very rarely and so being self-documenting is more important than being short. Changed (to 'NEEDS').

Another consideration is that each of these source annotations is itself a kind of API change, in the sense that it's not necessarily invisible to users. I'm thinking of how I have to specially tell both Doxygen and Netbeans IDE's code-assistance that these macros should be ignored (assumed empty) when parsing the code. But it seems that we accept this.

Committed revision 1543394.

- Julian
Received on 2013-11-19 12:39:42 CET