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

Re: svn_ra_dav__parsed_request error handling

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-08-05 21:44:06 CEST

Philip Martin wrote:
> Julian Foad <julianfoad@btopenworld.com> writes:
>
>>/Mais si/ - But yes, I think it is feasible and right for callers to
>>deal with the error return first, and only if that is null to then
>>examine outputs like the status code.
>
> As far as I can see there are circumstances where it is intentional
> that both an error is returned and status_code gets set.

Sorry - yes, I was totally unclear. I meant "The correct design would be ..."
rather than "The correct way to use the current implementation is ...".

> It's not clear to me that checking the error before the status_code is
> any more corect than checking status_code before the error. I suppose
> we could try to document which errors also have a status_code but
> strikes me as a poor interface.

Yes, that's what I meant; and, additionally, I assert that checking the error
return first is the normal convention and the function's implementation should
be altered, if necessary, to support that convention.

>>> if (baton.err)
[...]
>>Same sort of situation.
>
> I assume from your later comments that you are referring to baton. I
> think baton is different from status_code. status_code gets set, or
> not set, inside svn_ra_dav__parsed_request and the caller doesn't know
> exactly when it will happen and when status_code will be valid. On
> the other hand baton gets set in the callers callback, so it is
> entirely controlled by the caller and the caller can be expected to
> know when it is valid.

Oops, I completely missed that. Nothing wrong there, then. That situation is
true for all the user uses of the function, so the status code is the only issue.

[...]
> Not really. My suggestions are we do one of:
>
> * Have svn_ra_dav__parsed_request set *STATUS_CODE to zero (or -1 or
> some other magic number) before doing anything else

... AND document that *STATUS_CODE can be relied upon to have a meaningful
(non-garbage) value regardless whether the function returns an error, since
that is counter-intuitive.

> * Prominently document that the caller must always set *STATUS_CODE
> before calling svn_ra_dav__parsed_request

Well, actually, that this function won't necessarily set it, so the caller must
set it iff the caller wants it to be valid after an error return.

Equally distasteful to me, but either of these would be better than nothing.

> but I'd prefer an ra_dav expert to handle it.

Me too. If no-one volunteers within a couple of working days, I recommend we
leave the code alone and just document the current requirement.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 5 21:44:50 2005

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