trawick at gmail
Nov 18, 2009, 4:29 AM
Post #6 of 8
(788 views)
Permalink
|
|
Re: dav_new_error_*() and errno, revisit before 2.4 GA
[In reply to]
|
|
On Thu, Nov 12, 2009 at 7:04 AM, Jeff Trawick <trawick [at] gmail> wrote:
> On Wed, Nov 11, 2009 at 10:06 PM, Greg Stein <gstein [at] gmail> wrote:
>> On Wed, Nov 11, 2009 at 20:09, Jeff Trawick <trawick [at] gmail> wrote:
>>> On Wed, Nov 11, 2009 at 6:58 PM, Greg Stein <gstein [at] gmail> wrote:
>>>> On Wed, Nov 11, 2009 at 17:11, Jeff Trawick <trawick [at] gmail> 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/%3C20021101033848.B29006 [at] lyra%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?
|
