RA-Serf: misleading error message "Malformed URL"

A couple of improvement suggestions.

I noticed this in svn_ra_serf__get_lock():

> if (status_code == 404)
> {
> return svn_error_create(SVN_ERR_RA_ILLEGAL_URL, NULL,
> _("Malformed URL for repository"));
> }

If somebody hits this error, I expect they might be rather puzzled because their URL may not have been malformed, and they may not even have specified any URL at all on their original command. Better to (a) report the URL that failed, and (b) say more accurately what sort of error occurred.
And (c) doesn't this leak 'err' if 'err' is set and 'status_code' happens to contain 404 as well?

And immediately following that is this:

> if (err)
> {
> /* TODO Shh. We're telling a white lie for now. */
> return svn_error_create(SVN_ERR_RA_NOT_IMPLEMENTED, err,
> _("Server does not support locking features"));
> }

It would be better to be a bit more honest here as well, to make sure we don't waste the user's time when they go off trying to find out their server capabilities. Maybe say something like "A locking request failed for URL '%s'. Perhaps the server does not support locking features."

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-24 18:57:20 CEST