On Wed, Jan 25, 2006 at 12:14:57AM +0000, Julian Foad wrote:
> There are a few other functions where ignoring the return value guarantees
> a resource leak, but not many because of our wide use of pools.
That's a very good point. Okay, I've no objection to tying the two things
(type, attribute) together. In fact, I think it's probably better, since
it makes it harder to forget the attribute in the function definition.
> That just leaves Greg's point that my macro obscures the return value.
> Well, yes, it does. It wouldn't matter if we never referred to
> "svn_error_t" elsewhere, but we do, in this context:
>
> svn_error_t *err = fn(...);
>
> I was about to suggest another macro, something like SVN_ERR_TYPE to pair
> with SVN_ERR_FUNC, to be used here, but I don't think that would be an
> improvement.
>
Agreed. I don't think that would be an improvement either.
> >[That brings up and intreresting point actually: what does gcc < 3.4 do
> >with the attribute? Does it ignore it okay?]
>
> I imagine so. I had no idea this feature was so new. Could someone check
> that?
>
Just did. On gcc 3.3.6, you get:
foo.c:2: warning: `warn_unused_result' attribute directive ignored
So I'd think we'd to conditionalise on the gcc version.
> I think we should annotate everything - declarations and definitions -
> because that would be easier to understand and get right. GCC doesn't seem
> to care whether the definitions are annotated if the declarations are.
>
Good; simpler is better.
> Personally I find this style tolerable if necessary, but much worse than
> the style that used a single macro for "this returns an error object". I
> like the way it emphasised how this is our implementation of "throwing" an
> exception. Maybe we should indicate that with the macro name:
>
> SVN_THROW_FN svn_mime_type_validate (const char *mime_type,
> apr_pool_t *pool);
>
Please don't include 'throw' in the name, it's not C++. I think
SVN_ERR_FUNC was better at at least attempting to describe the return
value (plus, we don't really use 'throw' elsewhere).
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 25 13:06:08 2006