On Thu, Nov 12, 2009 at 7:04 AM, Jeff Trawick <[email protected]> wrote: > On Wed, Nov 11, 2009 at 10:06 PM, Greg Stein <[email protected]> wrote: >> On Wed, Nov 11, 2009 at 20:09, Jeff Trawick <[email protected]> wrote: >>> On Wed, Nov 11, 2009 at 6:58 PM, Greg Stein <[email protected]> wrote: >>>> On Wed, Nov 11, 2009 at 17:11, Jeff Trawick <[email protected]> wrote: >>>>> At present, whatever was in errno at the time the dav_error {} was >>>>> created is treated as an apr_status_t by ap_log_rerror(). >>>>> >>>>> http://mail-archives.apache.org/mod_mbox/httpd-dev/200211.mbox/%[email protected]%3e >>>>> >>>>> dav_error {} should have an apr_status_t field instead of an errno >>>>> field; functions which create a dav_error should have an apr_status_t >>>>> parameter. >>>>> >>>>> If there's no direct apr_status_t representation of the error, the >>>>> caller will have to decide what to do (no magic portable solution >>>>> AFAIK, but no worse than today). >>>>> >>>>> Concerns? >>>> >>>> dav_error was designed during the 1.3 series. APR wasn't even being >>>> discussed. So yeah... there is definitely a disconnect from "then" to >>>> "now". >>>> >>>> I'd be quite supportive of changing that, though (strictly speaking) >>>> that is probably an API change. >>> >>> well, yes it is an API change ;) I think the issue is whether or not >>> you in the DAV community want to push it on your third-party >>> colleagues. >>> >>> A third-party mod could define something like this for compatibility: >>> >>> #if AP_MODULE_MAGIC_AT_LEAST(xxx,y) >>> #define foo_dav_new_error(p, stat, errid, rv, desc) \ >>> dav_new_error(p, stat, errid, rv, desc) >>> #else >>> #define foo_dav_new_error(p, stat, errid, rv, desc) \ >>> (errno = (rv), \ >>> dav_new_error(p, stat, errid, desc)) >>> #endif >>> >>> and go ahead and change its code to pass in an apr_status_t. >>> >>> If it is too ugly to force this on third-party modules, we'll need to >>> add new APIs and use those ourselves, as it is too ugly for us to keep >>> mismanaging the error codes. >> >> I have no opinion in this space. I'm very much out of touch nowadays >> with the mod_dav users/community. The one particular user of mod_dav >> that I'm concerned with is mod_dav_svn, and *that* is about to become >> an Apache project :-) >> >> svn can certainly use that macro. "You" get to measure the disruption >> to other API users of Apache. > > Other folks will just have to recompile to accommodate the new MMN. > That has to be okay for pre-GA. > > Posting a heads-up rough patch to dav lists for other DAV modules will help. > >> In fact, we floated the idea of whether mod_dav_svn can/should be >> transferred over to httpd, rather than maintained as part of svn. >> Total random comment, but a couple people went "hmm...." It would be >> kinda neat for any Apache server to have built-in support for svn :-) > > shrug > >> (and would solve a bunch of versioning combinatorics for svn...) > > sure > > --/-- > > I'll post an errno->apr_status_t patch in the next day or two.
- For mod_dav in httpd trunk/future 2.4: http://people.apache.org/~trawick/mod_dav_err_api.txt The dav_new_error*() functions and dav_error structure already have status and error_id parameter/fields; what's a better choice than "aprerr" to replace the old "save_errno"? - For subversion modules: http://people.apache.org/~trawick/svn_err_api.txt svn already had its own dav_svn__new_error_tag() wrapper around dav_new_error_tag() to deal with errno issues, so I added another wrapper for dav_new_error(). I didn't add the apr_status_t value to the interface of the dav_svn wrapper, as it seemed that there was almost never an applicable apr_status_t handy to pass in. The MMN number in the #if isn't correct; that will be determined when the httpd patch is committed. --/-- Concerns (or "aprerr" replacement) before I commit the httpd changes? Does somebody want to fix any other mod_dav APIs in the same MMN change?
