Re: [PATCH] extend svn_io_file_* with calls for read(_full) and write(_full)

[ snip: yes I meant that ]

> 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.

Ok; it's the way some other functions in that header are documented too, but
I can fix all of them.

[snip]
> > + 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.

That is indeed the point, but the if(err) return err is an ooops. Good I
submitted for review...

> > 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.

I'll have a look at it and see if it needs lots of code to fix this.

Fixed commit comming up. Thanks for the review.

bye,

Erik.

-- 
Neu bei GMX: Preissenkung für MMS-Versand und FreeMMS!
Ideal für alle, die gerne MMS verschicken:
25 FreeMMS/Monat mit GMX TopMail.
http://www.gmx.net/de/cgi/produktemail
+++ GMX - die erste Adresse für Mail, Message, More! +++
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org