apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mihai Limbasan <mihai...@gmail.com>
Subject Re: apr_status_t testing against APR_SUCCESS usage question
Date Mon, 27 Dec 2004 14:47:17 GMT
I based my question on the fact that the majority of the cases seem to
explicitely check against the macro.
Oh well, should've waited before patching. It was a nice way of
getting the overall feel of the library though - nice work!


On Mon, 27 Dec 2004 09:44:06 -0500, Garrett Rooney
<rooneg@electricjellyfish.net> wrote:
> Mihai Limbasan wrote:
> >>From what I've gathered reviewing the APR code, the canonical way of
> > testing apr_* return values is comparing against APR_SUCCESS. That
> > happens to be #defined as zero.
> >
> > There are a lot of places around the APR codebase that do stuff like this:
> >
> >     if (rv)
> >         return rv;
> >
> > i.e. taking advantage of the knowledge that APR_SUCCESS is zero and
> > returning (or taking some other action) based on a non-zero (hence
> > non-APR_SUCCESS) status.
> >
> > This seems to defeat the point of having an APR_SUCCESS in the first
> > place. It's also (at least in my eyes) slightly less intuitive than
> > explicit testing.
> 
> Part of the definition of apr_status_t is the fact that APR_SUCCESS is
> defined to be zero.  This is by design, because success is the common
> case you test for, and comparing against zero is fast for most
> processors.  Because it will always be this way there's no reason not to
> publicise the fact, making the "if (rv) blah" idiom the common way to
> deal with errors in code that uses APR.
> 
> > Would it be useful if I went through the code and properly
> > canonicalized such constructs to
> >
> >     if (rv != APR_SUCCESS) {
> >         return rv;
> >     }
> >
> 
> Once you read the documentation and learn the idiom the "if (rv) return
> rv;" way is much easier to read IMO, I think your change would actually
> make things less clear in the long run.
> 
> -garrett
> 


-- 
Mihai Limbasan

Mime
View raw message