apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Garrett Rooney <roo...@electricjellyfish.net>
Subject Re: apr_status_t testing against APR_SUCCESS usage question
Date Mon, 27 Dec 2004 17:35:06 GMT
Cliff Woolley wrote:

> But because it is defined to be zero,
> 
> 
>>>    if (rv != APR_SUCCESS) {
>>>        return rv;
>>>    }
>>>
> 
> 
> will be equivalent to and just as fast as
> 
> 
>>>    if (!rv) {
>>>        return rv;
>>>    }
> 
> 
> on any remotely reasonable compiler. 

Ok, I think we may be talking about two different cases...

There's the "check if a call returned an error, and if so return that to 
your caller" case, which personally I think is easist to read as:

if (rv)
   return rv;

If there is an error, we return it, the code does exactly what you would 
read out loud if you were reading it to someone.  In Subversion land we 
make it even more explicit by using 'err' or 'apr_err' for the variable 
holding the status, instead of rv, but even without that it's still more 
readable this way IMO.

And there's the "check if a call succeeded, and if so return that to 
your caller" case, which I'm not all that picky about.  Perhaps 
including APR_SUCCESS there in some manner is probably best, as it makes 
it clear that we're looking for success.

The patch seems to deal with both situations, and in the first case I 
just don't see how it helps other than making things more verbose when 
there's little need.

-garrett

Mime
View raw message