Re: using APR_STATUS_IS_SUCCESS
Geoffrey Young wrote: hi all I was just in garrett's APR talk here at oscon and he was mentioning the APR_STATUS_IS_SUCCESS macro, which I found interesting since httpd only uses it in a few places, opting for a direct comparison to APR_SUCCESS instead. should we move to APR_STATUS_IS_SUCCESS in all places? can someone who groks the nuances of the macro add some insight here? This is actually something I was wondering about as I wrote the presentation. Neither Apache or Subversion use APR_STATUS_IS_SUCCESS everywhere, but I wonder if we should, since if you look at the definition of the macro there are cases when it's more than just (s) == APR_SUCCESS. -garrett
Re: using APR_STATUS_IS_SUCCESS
cross-posted to [EMAIL PROTECTED] Garrett Rooney wrote: Geoffrey Young wrote: hi all I was just in garrett's APR talk here at oscon and he was mentioning the APR_STATUS_IS_SUCCESS macro, which I found interesting since httpd only uses it in a few places, opting for a direct comparison to APR_SUCCESS instead. should we move to APR_STATUS_IS_SUCCESS in all places? can someone who groks the nuances of the macro add some insight here? This is actually something I was wondering about as I wrote the presentation. Neither Apache or Subversion use APR_STATUS_IS_SUCCESS everywhere, but I wonder if we should, since if you look at the definition of the macro there are cases when it's more than just (s) == APR_SUCCESS. just another note, I grep'd the code for rc == APR_SUCCESS and it looks like not even APR is using the macro everywhere... --Geoff
Re: using APR_STATUS_IS_SUCCESS
The initial thought was you might have LDAP success, OS status success, and possibly multiple return codes that were considered successes. Nothing was ever done with this. Bill At 02:40 PM 7/28/2004, Garrett Rooney wrote: Geoffrey Young wrote: hi all I was just in garrett's APR talk here at oscon and he was mentioning the APR_STATUS_IS_SUCCESS macro, which I found interesting since httpd only uses it in a few places, opting for a direct comparison to APR_SUCCESS instead. should we move to APR_STATUS_IS_SUCCESS in all places? can someone who groks the nuances of the macro add some insight here? This is actually something I was wondering about as I wrote the presentation. Neither Apache or Subversion use APR_STATUS_IS_SUCCESS everywhere, but I wonder if we should, since if you look at the definition of the macro there are cases when it's more than just (s) == APR_SUCCESS. -garrett
Re: using APR_STATUS_IS_SUCCESS
William A. Rowe, Jr. wrote: The initial thought was you might have LDAP success, OS status success, and possibly multiple return codes that were considered successes. Nothing was ever done with this. What about the win32 definition of the macro: #define APR_STATUS_IS_SUCCESS(s) ((s) == APR_SUCCESS \ || (s) == APR_OS_START_SYSERR + ERROR_SUCCESS) If you just compare against APR_SUCCESS won't you miss the ERROR_SUCCESS part there? -garrett
Re: using APR_STATUS_IS_SUCCESS
On Wed, Jul 28, 2004 at 08:08:05PM -0400, Ryan Bloom wrote: Basically, the macro is wrong and needs to be removed. The contract that _all_ APR API's live up to is that on a successful result, they must return APR_SUCCESS. The reason we chose to use 0 as success is simple: Yup. The contract is that success is 0. There is no system success -- APR should translate any/all underlying success values into 0 upon return. So I fully ocncur with the original design contract -- both in terms of what the history and design said it should be, and how I (still) think it should operate. I'm with Ryan on calling that macro wrong, and just tossing it. We tossed its use from SVN a long time ago and just relied on the ==0 contract. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: using APR_STATUS_IS_SUCCESS
Greg Stein wrote:
On Wed, Jul 28, 2004 at 08:08:05PM -0400, Ryan Bloom wrote:
Basically, the macro is wrong and needs to be removed. The contract
that _all_ APR API's live up to is that on a successful result, they
must return APR_SUCCESS. The reason we chose to use 0 as success is
simple:
Yup. The contract is that success is 0. There is no system success --
APR should translate any/all underlying success values into 0 upon return.
So I fully ocncur with the original design contract -- both in terms of
what the history and design said it should be, and how I (still) think it
should operate.
I'm with Ryan on calling that macro wrong, and just tossing it. We tossed
its use from SVN a long time ago and just relied on the ==0 contract.
$ find . -name \*.c -exec grep -q APR_STATUS_IS_SUCCESS {} \; -print
./subversion/libsvn_fs_fs/fs_fs.c
./subversion/libsvn_ra_svn/cram.c
./subversion/libsvn_ra_svn/marshal.c
./subversion/libsvn_subr/io.c
./subversion/tests/libsvn_delta/svndiff-test.c
./subversion/tests/libsvn_delta/vdelta-test.c
$
Apparently it's crept back in.
-garrett
Re: using APR_STATUS_IS_SUCCESS
Basically, the macro is wrong and needs to be removed. The contract that _all_ APR API's live up to is that on a successful result, they must return APR_SUCCESS. The reason we chose to use 0 as success is simple: 1) Most platforms can check for equality with 0 faster than they can check for any other integer equality. 2) It makes checking for success _really_ easy, because all APIs use the same value for success, there is no guessing or research needed, if the result wasn't 0, then the function didn't succeed. It didn't necessarily fail, because there are status codes that aren't full success and aren't failures, but more research is needed. 3) It provides you an opportunity to have a lot of different values for errors and statuses without having to use a separate variable. I assumed that the original addition of the macro was so that success was handled like any other result code, ie you always use a macro. If the reason was so that some functions could return non-zero success codes, then the macro definately needs to go, because that is a really bad idea. Ryan On Wed, 28 Jul 2004 13:47:20 -0700, Geoffrey Young [EMAIL PROTECTED] wrote: cross-posted to [EMAIL PROTECTED] Garrett Rooney wrote: Geoffrey Young wrote: hi all I was just in garrett's APR talk here at oscon and he was mentioning the APR_STATUS_IS_SUCCESS macro, which I found interesting since httpd only uses it in a few places, opting for a direct comparison to APR_SUCCESS instead. should we move to APR_STATUS_IS_SUCCESS in all places? can someone who groks the nuances of the macro add some insight here? This is actually something I was wondering about as I wrote the presentation. Neither Apache or Subversion use APR_STATUS_IS_SUCCESS everywhere, but I wonder if we should, since if you look at the definition of the macro there are cases when it's more than just (s) == APR_SUCCESS. just another note, I grep'd the code for rc == APR_SUCCESS and it looks like not even APR is using the macro everywhere... --Geoff -- Ryan Bloom [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED]
