[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r35328 - in trunk/subversion/mod_dav_svn: . reports

From: Greg Stein <gstein_at_gmail.com>
Date: Wed, 21 Jan 2009 19:46:13 +0100

On Wed, Jan 21, 2009 at 17:41, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> Greg Stein wrote:
>> On Mon, Jan 19, 2009 at 18:40, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>> ...
>>> +++ trunk/subversion/mod_dav_svn/reports/get-locks.c Mon Jan 19 09:40:49 2009 (r35328)
>>> ...
>>> + if ((apr_err = send_get_lock_response(locks, output, bb, resource->pool)))
>>> + derr = dav_svn__convert_err(svn_error_create(apr_err, 0, NULL),
>>> HTTP_INTERNAL_SERVER_ERROR,
>>> - "Error flushing brigade.",
>>> + "Error writing REPORT response.",
>>> resource->pool);
>>
>> Gah. Use dav_new_error() please.
>
> I'm looking at dav_new_error(), and I think there's an information loss to
> be had if I use that:
>
> dav_new_error() accepts:
> - the HTTP status code
> - what appears to be a magic DAV error code (which we always pass as
> 0 from mod_dav_svn)

That should be serr->apr_err

> - the error string
> - a pool
>
> dav_svn__convert_err() accepts:
> - the HTTP status code
> - an svn_error_t *, from which is made a dav_error chain that seems to
> preserve apr_err)

Yup. The conversion uses ->apr_err for the error_id field.

> - the error string
> - a pool
>
> So, by using the former, we appear to be losing a useful bit of information:
> the original apr_status_t value. Am I misunderstanding something?

Nominally, error_id was designed as one of the enumerated values in
mod_dav.h, but bleh... using apr_err works just fine.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1041739
Received on 2009-01-21 19:46:52 CET

This is an archived mail posted to the Subversion Dev mailing list.