On Tue, Jan 24, 2006 at 12:38:58PM +0000, Julian Foad wrote:
> Most of our functions (over 1200 of them) return "svn_error_t *", and I
> think writing "__attribute__((warn_unused_result))" literally on every such
> function would be far too ugly.
Agreed.
> Yes, I know (now that Philip reminded me)
> that we have "__attribute__((format(printf,...))" on some functions, but
> only a dozen or so. Certainly, if someone suggested adding such a long,
> compiler-specific annotation for, say, a Microsoft compiler, to a large
> number of functions, I would be one of the first to object. Therefore I
> suggest this instead:
>
> +#define SVN_ERR_FUNC __attribute__((warn_unused_result)) svn_error_t *
Nice, and I like the attention to detail wrt the indentation. However,
as Greg points out, this does obscure the return value (and who's to
say we wouldn't want to use this with other return types?
We could do something similar to the Linux kernel, and define a keyword
that expands to the syntax we need.
LXR has a copy of the 2.6.11 source, which declares (in a GCC3-specific file):
#if __GNUC_MINOR__ >= 4
#define __must_check __attribute__((warn_unused_result))
#endif
[http://lxr.linux.no/source/include/linux/compiler-gcc3.h]
__must_check is later conditionally defined nothing if it's not yet
been defined.
[That brings up and intreresting point actually: what does gcc < 3.4 do
with the attribute? Does it ignore it okay?]
Then again, I see the value of making it hard to declare a function
returning an svn_error_t* without also declaring it __must_check or
equivalent.
> I do NOT believe it is very
> important to try to make other compilers do this same checking. The more
> checks the better, but some compilers will always check things that others
> don't, and that's fine.
Agreed, and that's why I'm not too bothered about hard-coding the position
of the token.
> How do people feel about me committing this? The patch I have ready covers
> all (1200 or so) declarations in header files. I suppose I should do the
> same for all the (1700 or so) "static" functions too - leaks from them are
> no less important.
>
Do we annotate just the declarations (in public header files and statics),
and also all the static definitions that lack declarations? Or do we just
annotate everything in sight, it being easier? Does it matter to gcc?
Do we bump the copyright dates on all the files too? (With my IANAL
hat firmly on, I'd guess not: this is a mechanical change, not one that
introduces anything new).
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 24 22:32:51 2006