+1 on the first patch. Re the second patch: I'm neutral. The default
error message for SVN_ERR_RA_SVN_CONNECTION_CLOSED is already fine,
IMHO.
I infer that you didn't commit this either because you wanted comments
first, or perhaps were on the way out the door and didn't have time to
run 'make svnserveautocheck'? If the latter, I have done so (with just
the first patch applied), and all tests pass, so feel free to commit :-).
-Karl
epg_at_google.com writes:
> [[[
> Don't use an error message with SVN_ERR_RA_SVN_CONNECTION_CLOSED when the
> error message adds nothing.
>
> * subversion/libsvn_ra_svn/streams.c
> (file_read_cb, sock_read_cb): Use NULL for error message.
>
> * subversion/libsvn_ra_svn/marshal.c
> (readbuf_input): Use NULL for error message.
> ]]]
>
> Index: subversion/libsvn_ra_svn/streams.c
> ===================================================================
> --- subversion/libsvn_ra_svn/streams.c (revision 30764)
> +++ subversion/libsvn_ra_svn/streams.c (working copy)
> @@ -76,8 +76,7 @@ file_read_cb(void *baton, char *buffer, apr_size_t
> if (status && !APR_STATUS_IS_EOF(status))
> return svn_error_wrap_apr(status, _("Can't read from connection"));
> if (*len == 0)
> - return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL,
> - _("Connection closed unexpectedly"));
> + return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL, NULL);
> return SVN_NO_ERROR;
> }
>
> @@ -151,8 +150,7 @@ sock_read_cb(void *baton, char *buffer, apr_size_t
> if (status && !APR_STATUS_IS_EOF(status))
> return svn_error_wrap_apr(status, _("Can't read from connection"));
> if (*len == 0)
> - return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL,
> - _("Connection closed unexpectedly"));
> + return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL, NULL);
> return SVN_NO_ERROR;
> }
>
> Index: subversion/libsvn_ra_svn/marshal.c
> ===================================================================
> --- subversion/libsvn_ra_svn/marshal.c (revision 30764)
> +++ subversion/libsvn_ra_svn/marshal.c (working copy)
> @@ -247,8 +247,7 @@ static svn_error_t *readbuf_input(svn_ra_svn_conn_
>
> SVN_ERR(svn_ra_svn__stream_read(conn->stream, data, len));
> if (*len == 0)
> - return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL,
> - _("Connection closed unexpectedly"));
> + return svn_error_create(SVN_ERR_RA_SVN_CONNECTION_CLOSED, NULL, NULL);
>
> if (session)
> {
>
> And, if people prefer the way the error messages were phrased at
> the creation site, I can include this. Otherwise, I won't bother.
>
> [[[
> * subversion/include/svn_error_codes.h
> (SVN_ERR_RA_SVN_CONNECTION_CLOSED): Rephrase error message.
> ]]]
>
> Index: subversion/include/svn_error_codes.h
> ===================================================================
> --- subversion/include/svn_error_codes.h (revision 30764)
> +++ subversion/include/svn_error_codes.h (working copy)
> @@ -818,7 +818,7 @@ SVN_ERROR_START
>
> SVN_ERRDEF(SVN_ERR_RA_SVN_CONNECTION_CLOSED,
> SVN_ERR_RA_SVN_CATEGORY_START + 2,
> - "Network connection closed unexpectedly")
> + "Connection closed unexpectedly")
>
> SVN_ERRDEF(SVN_ERR_RA_SVN_IO_ERROR,
> SVN_ERR_RA_SVN_CATEGORY_START + 3,
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-24 01:24:38 CEST