You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2013/07/14 20:07:55 UTC

svn commit: r1503010 - /subversion/trunk/subversion/libsvn_subr/utf.c

Author: danielsh
Date: Sun Jul 14 18:07:54 2013
New Revision: 1503010

URL: http://svn.apache.org/r1503010
Log:
svn_utf_*: Improve error handling.

This is part of a patch I posted recently.  The remaining part --- handling
APR_EINVAL and APR_ENOTIMPL as fatal errors --- may be committed separately
(for ease of backporting).

* subversion/libsvn_subr/utf.c
  (xlate_alloc_handle): Tweak the error handling so that apr_strerror() of the
    error code is included in the error chain, in addition to the custom error
    added here.  The incumbent code would report error codes such as E70023,
    but mere mortals aren't supposed to know what errno error that maps to,
    and that is useful information:

      svn: E070023: Can't create a character converter from native encoding to 'UTF-8'
      svn: E070023: This function has not been implemented on this platform

Modified:
    subversion/trunk/subversion/libsvn_subr/utf.c

Modified: subversion/trunk/subversion/libsvn_subr/utf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/utf.c?rev=1503010&r1=1503009&r2=1503010&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/utf.c (original)
+++ subversion/trunk/subversion/libsvn_subr/utf.c Sun Jul 14 18:07:54 2013
@@ -233,6 +233,8 @@ xlate_alloc_handle(xlate_handle_node_t *
   else if (apr_err != APR_SUCCESS)
     {
       const char *errstr;
+      char apr_strerr[512];
+
       /* Can't use svn_error_wrap_apr here because it calls functions in
          this file, leading to infinite recursion. */
       if (frompage == SVN_APR_LOCALE_CHARSET)
@@ -248,7 +250,13 @@ xlate_alloc_handle(xlate_handle_node_t *
                               _("Can't create a character converter from "
                                 "'%s' to '%s'"), frompage, topage);
 
-      return svn_error_create(apr_err, NULL, errstr);
+      /* Just put the error on the stack, since svn_error_create duplicates it
+         later.  APR_STRERR will be in the local encoding, not in UTF-8, though.
+       */
+      svn_strerror(apr_err, apr_strerr, sizeof(apr_strerr));
+      return svn_error_create(apr_err, 
+                              svn_error_create(apr_err, NULL, apr_strerr),
+                              errstr);
     }
 
   /* Allocate and initialize the node. */



Re: svn commit: r1503010 - /subversion/trunk/subversion/libsvn_subr/utf.c

Posted by Daniel Shahaf <da...@apache.org>.
Philip Martin wrote on Mon, Jul 15, 2013 at 11:22:27 +0100:
> danielsh@apache.org writes:
> 
> > Author: danielsh
> > Date: Sun Jul 14 18:07:54 2013
> > New Revision: 1503010
> >
> > URL: http://svn.apache.org/r1503010
> > Log:
> > svn_utf_*: Improve error handling.
> >
> > This is part of a patch I posted recently.  The remaining part --- handling
> > APR_EINVAL and APR_ENOTIMPL as fatal errors --- may be committed separately
> > (for ease of backporting).
> >
> > * subversion/libsvn_subr/utf.c
> >   (xlate_alloc_handle): Tweak the error handling so that apr_strerror() of the
> >     error code is included in the error chain, in addition to the custom error
> >     added here.  The incumbent code would report error codes such as E70023,
> >     but mere mortals aren't supposed to know what errno error that maps to,
> >     and that is useful information:
> >
> >       svn: E070023: Can't create a character converter from native encoding to 'UTF-8'
> >       svn: E070023: This function has not been implemented on this platform
> 
> Is that an example of the old output, or the new output, or something
> else?  E070023 is ENOTIMPL and xlate_alloc_handle catches that error and
> so will not produce any error output.

That is example of how the output looks in combination with the patch to
s/else if/if/.  It's representative insofar as 70023 is APR_ENOTIMPL and
the last element of the error chain is apr_strerror(ENOTIMPL), but you
are correct that xlate_alloc_handle() can't produce it in HEAD.

Re: svn commit: r1503010 - /subversion/trunk/subversion/libsvn_subr/utf.c

Posted by Philip Martin <ph...@wandisco.com>.
danielsh@apache.org writes:

> Author: danielsh
> Date: Sun Jul 14 18:07:54 2013
> New Revision: 1503010
>
> URL: http://svn.apache.org/r1503010
> Log:
> svn_utf_*: Improve error handling.
>
> This is part of a patch I posted recently.  The remaining part --- handling
> APR_EINVAL and APR_ENOTIMPL as fatal errors --- may be committed separately
> (for ease of backporting).
>
> * subversion/libsvn_subr/utf.c
>   (xlate_alloc_handle): Tweak the error handling so that apr_strerror() of the
>     error code is included in the error chain, in addition to the custom error
>     added here.  The incumbent code would report error codes such as E70023,
>     but mere mortals aren't supposed to know what errno error that maps to,
>     and that is useful information:
>
>       svn: E070023: Can't create a character converter from native encoding to 'UTF-8'
>       svn: E070023: This function has not been implemented on this platform

Is that an example of the old output, or the new output, or something
else?  E070023 is ENOTIMPL and xlate_alloc_handle catches that error and
so will not produce any error output.

>
> Modified:
>     subversion/trunk/subversion/libsvn_subr/utf.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/utf.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/utf.c?rev=1503010&r1=1503009&r2=1503010&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/utf.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/utf.c Sun Jul 14 18:07:54 2013
> @@ -233,6 +233,8 @@ xlate_alloc_handle(xlate_handle_node_t *
>    else if (apr_err != APR_SUCCESS)
>      {
>        const char *errstr;
> +      char apr_strerr[512];
> +
>        /* Can't use svn_error_wrap_apr here because it calls functions in
>           this file, leading to infinite recursion. */
>        if (frompage == SVN_APR_LOCALE_CHARSET)
> @@ -248,7 +250,13 @@ xlate_alloc_handle(xlate_handle_node_t *
>                                _("Can't create a character converter from "
>                                  "'%s' to '%s'"), frompage, topage);
>  
> -      return svn_error_create(apr_err, NULL, errstr);
> +      /* Just put the error on the stack, since svn_error_create duplicates it
> +         later.  APR_STRERR will be in the local encoding, not in UTF-8, though.
> +       */
> +      svn_strerror(apr_err, apr_strerr, sizeof(apr_strerr));
> +      return svn_error_create(apr_err, 
> +                              svn_error_create(apr_err, NULL, apr_strerr),
> +                              errstr);
>      }
>  
>    /* Allocate and initialize the node. */
>
>

-- 
Philip Martin | Subversion Committer
WANdisco | Non-Stop Data
www.wandisco.com