On Sun, 2003-11-30 at 04:08, Erik Huelsmann wrote:
> I was about to commit this change when Greg Hudson said he would not have
> coded it this way, so I'm submitting for review.
Er, I think what Erik meant is: he wrote something, I said it didn't
look right to me, we talked about it and came up with something new, but
based on that he wants review on the finished product.
Three comments:
> +/** Wrapper for @c apr_file_read(), which see. */
I guess by "which see," you mean, "see the documentation for that
function," but I don't think that's very clear.
> if (fname)
> - SVN_ERR (svn_path_cstring_to_utf8 (fname_utf8, fname, pool));
> - else
> - *fname_utf8 = NULL;
> + {
> + svn_error_t *err;
>
> + err = svn_path_cstring_to_utf8 (&fname, fname, pool);
> + if (err)
> + return err;
> + }
> + *fname_utf8 = fname;
> return SVN_NO_ERROR;
I guess the point here is to guarantee that *fname_utf8 doesn't get
garbage of UTF-8 conversion fails. But you also wrote out an SVN_ERR()
the long way; SVN_ERR(svn_path_cstring_to_utf8 (&fname, fname, pool))
should do fine.
> if (status)
> - return svn_error_createf (status, NULL,
> - "svn_io_file_open: can't open '%s'", fname);
> + {
> + apr_strerror (status, errbuf, sizeof(errbuf));
> + SVN_ERR (svn_utf_cstring_to_utf8 (&err_str, errbuf, pool));
> + return svn_error_createf
> + (status, NULL, "Can't open file '%s': %s", fname, err_str);
> + }
The behavior on error is different from that of the IO wrapper. But
then, UTF-8 conversion of an APR error string is never going to fail, so
perhaps this isn't worth worrying about.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 30 17:42:49 2003