mark benedetto king <bking@inquira.com> writes:
> Nevermind, I found it...it was issue 900, permissions on ra_local, not
> ra_dav. Though *that* patch is still uncommitted. :-)
Notice, however, that it *is* in a milestone :-).
Some comments on the patch attached there:
> Shepherd the ENOENT from apr_file_open up through the call
> stack to svn_ra_local__split_URL. Accomplish this via
> empassioned pleas to maintenance programmers not to break
> this frail exception handling mechanism.
Those pleas are written in internal comments, not in the public
interfaces. But the pleas *are* essentially interface requirements,
so they need to be made explicit, or else it will never be
maintainable.
Here's the patch, for reference:
> * subversion/include/svn_error_codes.h:
> SVN_ERR_RA_LOCAL_REPOS_OPEN_FAILED: new error code.
>
> * subversion/libsvn_subr/io.c:
> (svn_io_file_open): Added comment.
> (svn_io_read_version_file): Added comment.
>
> * subversion/libsvn_ra_local/split_url.c
> (svn_ra_local__split_URL): Handle errors other
> than missing repository.
>
>
>
> Index: subversion/include/svn_error_codes.h
> ===================================================================
> --- subversion/include/svn_error_codes.h
> +++ subversion/include/svn_error_codes.h Mon Sep 16 17:27:16 2002
> @@ -519,6 +519,10 @@
> SVN_ERR_RA_LOCAL_CATEGORY_START + 0,
> "Couldn't find a repository.")
>
> + SVN_ERRDEF (SVN_ERR_RA_LOCAL_REPOS_OPEN_FAILED,
> + SVN_ERR_RA_LOCAL_CATEGORY_START + 1,
> + "Couldn't open a repository.")
> +
> /* svndiff errors */
>
> SVN_ERRDEF (SVN_ERR_SVNDIFF_INVALID_HEADER,
> Index: subversion/libsvn_subr/io.c
> ===================================================================
> --- subversion/libsvn_subr/io.c
> +++ subversion/libsvn_subr/io.c Mon Sep 16 16:57:03 2002
> @@ -1568,6 +1568,9 @@
> SVN_ERR (svn_utf_cstring_from_utf8 (&fname_native, fname, pool));
> status = apr_file_open (new_file, fname_native, flag, perm, pool);
>
> + /* The exact status code from apr_file_open is checked by
> + at least one caller (svn_io_read_version_file). This
> + status MUST be preserved. */
> if (status)
> return svn_error_createf (status, 0, NULL, pool,
> "svn_io_file_open: can't open `%s'", fname);
> @@ -1911,7 +1914,10 @@
> svn_stringbuf_t *version_str;
> apr_status_t apr_err;
>
> - /* Read a line from PATH */
> + /* Read a line from PATH.
> + The exact status code from svn_io_file_open is checked
> + by at least one caller (svn_repos_open). Any error MUST
> + be passed on, unaltered. */
> SVN_ERR (svn_io_file_open (&format_file, path, APR_READ,
> APR_OS_DEFAULT, pool));
> format_stream = svn_stream_from_aprfile (format_file, pool);
> Index: subversion/libsvn_ra_local/split_url.c
> ===================================================================
> --- subversion/libsvn_ra_local/split_url.c
> +++ subversion/libsvn_ra_local/split_url.c Mon Sep 16 17:26:39 2002
> @@ -111,6 +111,12 @@
> if (err == SVN_NO_ERROR)
> break;
>
> + if (!APR_STATUS_IS_ENOENT(err->apr_err))
> + return svn_error_createf
> + (SVN_ERR_RA_LOCAL_REPOS_OPEN_FAILED, 0, err, pool,
> + "svn_ra_local__split_URL: Unable to open repository\n"
> + " (%s)", URL);
> +
> /* It would be strange indeed if "/" were a repository, but hey,
> people do strange things sometimes. Anyway, if "/" failed
> the test above, then reduce it to the empty string.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Oct 28 06:41:46 2002