Bert Huijben wrote: >> Bert Huijben wrote: >>> Julian Foad wrote: >>>> 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. >> >> It is just compiler magic and doesn't affect the ABI or API. [...]
I was referring to the change of SVN_NO_ERROR from '0' to '(void *)0'. That's a change of public API. Third-party users will see that change. (It's not an ABI change of course, because preprocessor macros aren't exposed in the ABI.) If a third party is just using SVN_NO_ERROR in the intended way, this will be a good change. If they are using it in an unintended way (where some non-svn_error_t zero value is wanted) then they will henceforth potentially get a warning or an error or possibly even a change of behaviour. But now I see that the real issue with svn_cl__try is its variable arguments are not pointers at all, but error codes of (integer) type 'apr_status_t'. So the sentinel should be 0 or APR_SUCCESS. I've changed the sentinels there to 0 and updated the doc string, in 1543507. Earlier, in r1543477, I updated all remaining variadic function calls that want a pointer-null sentinel, to use SVN_VA_NULL instead of plain NULL. - Julian
