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

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.

The patch is also available
at:
http://encorps.dnsalias.com/svn-public/patches/897-libsvn_subr-apr_file-wrappers.log-patch

Comments please?

Log:
[[[
* subversion/libsvn_subr/io.c:
  (svn_io_file_read,svn_io_file_read_full,
   svn_io_file_write,svn_io_file_write_full):
   New. Wrappers around apr_file_ calls returning svn_error_t's.
  (do_io_file_wrapper_cleanup):
   New. Implements cleanup procedure for the file wrappers above.
  (file_name_get): Ensure that NULL is set on error
   in *fname_utf8: a requirement for do_io_file_wrapper_cleanup.
  (svn_io_file_open): Return an error string formatted like the ones
   from do_io_file_wrapper_cleanup.
  (svn_io_file_close): Use do_io_file_wrapper_cleanup for
   error handling.

* subversion/include/svn_io.h:
  Add declarations for svn_io_file_read(_full) and
  svn_io_file_write(_full) functions.
]]]

Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 7872)
+++ subversion/include/svn_io.h (working copy)
@@ -644,6 +644,32 @@
 svn_io_file_close (apr_file_t *file, apr_pool_t *pool);
 
 
+/** Wrapper for @c apr_file_read(), which see. */
+svn_error_t *
+svn_io_file_read (apr_file_t *file, void *buf,
+ apr_size_t *nbytes, apr_pool_t *pool);
+
+
+/** Wrapper for @c apr_file_write_full(), which see. */
+svn_error_t *
+svn_io_file_read_full (apr_file_t *file, void *buf,
+ apr_size_t nbytes, apr_size_t *bytes_read,
+ apr_pool_t *pool);
+
+
+/** Wrapper for @c apr_file_write(), which see. */
+svn_error_t *
+svn_io_file_write (apr_file_t *file, void *buf,
+ apr_size_t *nbytes, apr_pool_t *pool);
+
+
+/** Wrapper for @c apr_file_write_full(), which see. */
+svn_error_t *
+svn_io_file_write_full (apr_file_t *file, void *buf,
+ apr_size_t nbytes, apr_size_t *bytes_written,
+ apr_pool_t *pool);
+
+
 /** Wrapper for @c apr_stat(), which see. @a fname is utf8-encoded. */
 svn_error_t *
 svn_io_stat (apr_finfo_t *finfo, const char *fname,
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 7872)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -973,13 +973,17 @@
 }
 
 
-/* Get the name of FILE, or NULL if FILE is an unnamed stream. */
+/* Get the name of FILE, or NULL if FILE is an unnamed stream.
+ On error *fname_utf8 is guaranteed to contain NULL.
+ */
 static svn_error_t *
 file_name_get (const char **fname_utf8, apr_file_t *file, apr_pool_t *pool)
 {
   apr_status_t apr_err;
   const char *fname;
 
+ *fname_utf8 = NULL;
+
   apr_err = apr_file_name_get (&fname, file);
   if (apr_err)
     return svn_error_create
@@ -987,10 +991,15 @@
        "failed to get file name from APR");
 
   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;
 }
 
@@ -1685,44 +1694,100 @@
                   apr_int32_t flag, apr_fileperms_t perm,
                   apr_pool_t *pool)
 {
+ const char *err_str;
   const char *fname_apr;
   apr_status_t status;
+ char errbuf[255];
 
   SVN_ERR (svn_path_cstring_from_utf8 (&fname_apr, fname, pool));
   status = apr_file_open (new_file, fname_apr, flag, perm, pool);
 
   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);
+ }
   else
     return SVN_NO_ERROR;
 }
 
 
+static svn_error_t *
+do_io_file_wrapper_cleanup (apr_file_t *file, apr_status_t status,
+ const char *op, apr_pool_t *pool)
+{
+ const char *name;
+ const char *errstr;
+ char errbuf[255];
+ svn_error_t *err;
+
+ if (! status)
+ return SVN_NO_ERROR;
+
+ svn_error_clear (file_name_get (&name, file, pool));
+ name = (name) ? apr_psprintf (pool, "file '%s'", name) : "stream";
+ apr_strerror (status, errbuf, sizeof(errbuf));
+ err = svn_utf_cstring_to_utf8 (&errstr, errbuf, pool);
+ errstr = (err) ? "" : apr_psprintf (pool, ": %s", errstr);
+ svn_error_clear (err);
+
+ return svn_error_createf (status, NULL, "Can't %s %s%s", op, name,
errstr);
+}
+
 svn_error_t *
 svn_io_file_close (apr_file_t *file, apr_pool_t *pool)
 {
- apr_status_t status;
+ return do_io_file_wrapper_cleanup
+ ( file, apr_file_close (file),
+ "close", pool);
+}
 
- status = apr_file_close (file);
 
- if (status)
- {
- const char *fname_utf8;
- SVN_ERR (file_name_get (&fname_utf8, file, pool));
-
- if (NULL == fname_utf8)
- fname_utf8 = "(stdin/out/err?)";
+svn_error_t *
+svn_io_file_read (apr_file_t *file, void *buf,
+ apr_size_t *nbytes, apr_pool_t *pool)
+{
+ return do_io_file_wrapper_cleanup
+ ( file, apr_file_read (file, buf, nbytes),
+ "read", pool);
+}
 
- return svn_error_createf (status, NULL,
- "svn_io_file_close: can't close '%s'",
- fname_utf8);
- }
- return SVN_NO_ERROR;
+
+svn_error_t *
+svn_io_file_read_full (apr_file_t *file, void *buf,
+ apr_size_t nbytes, apr_size_t *bytes_read,
+ apr_pool_t *pool)
+{
+ return do_io_file_wrapper_cleanup
+ ( file, apr_file_read_full (file, buf, nbytes, bytes_read),
+ "read", pool);
 }
 
 
 svn_error_t *
+svn_io_file_write (apr_file_t *file, void *buf,
+ apr_size_t *nbytes, apr_pool_t *pool)
+{
+ return do_io_file_wrapper_cleanup
+ ( file, apr_file_write (file, buf, nbytes),
+ "read", pool);
+}
+
+
+svn_error_t *
+svn_io_file_write_full (apr_file_t *file, void *buf,
+ apr_size_t nbytes, apr_size_t *bytes_written,
+ apr_pool_t *pool)
+{
+ return do_io_file_wrapper_cleanup
+ ( file, apr_file_write_full (file, buf, nbytes, bytes_written),
+ "read", pool);
+}
+
+
+svn_error_t *
 svn_io_stat (apr_finfo_t *finfo, const char *fname,
              apr_int32_t wanted, apr_pool_t *pool)
 {

-- 
HoHoHo! Seid Ihr auch alle schön brav gewesen?
GMX Weihnachts-Special: Die 1. Adresse für Weihnachts-
männer und -frauen! http://www.gmx.net/de/cgi/specialmail
+++ 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