[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] issue #897: libsvn_client

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2003-11-06 05:11:46 CET

Branko Čibej wrote:
> Erik Huelsmann wrote:
>>
>>My first revision of a patch for libsvn_client cf issue 897.
>
> There are a few more things that could be done for consistency.
...
> * Message texts
> o Close-file errors: "Failed to close file '%s'" vs. "Error
> closing file '%s'"

How about I make the following addition so that most invocations of apr_file_close can be changed to SVN_ERR (svn_io_file_close) instead of being followed by manual construction of an error message?

- Julian

[[[
Add a wrapper around apr_file_close to allow more concise error handling.

The new function svn_io_file_close complements svn_io_file_open and returns
an svn_error_t so that errors can be dealt with by use of SVN_ERR rather than
by manual construction of an error message at each call site.

* subversion/include/svn_io.h
  (svn_io_file_close): New wrapper around apr_file_close.

* subversion/libsvn_subr/io.c
  (file_name_get): New helper function.
  (svn_stringbuf_from_aprfile): Use file_name_get.
  (svn_io_file_close): New function, also using file_name_get.
]]]

Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 7646)
+++ subversion/include/svn_io.h (working copy)
@@ -636,6 +636,11 @@
                   apr_pool_t *pool);

+/** Wrapper for @c apr_file_close(), which see. */
+svn_error_t *
+svn_io_file_close (apr_file_t *file, 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 7646)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -993,6 +993,28 @@
 }

+/* Get the name of FILE, or NULL if FILE is an unnamed stream. */
+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;
+
+ apr_err = apr_file_name_get (&fname, file);
+ if (apr_err)
+ return svn_error_create
+ (apr_err, NULL,
+ "failed to get file name from APR");
+
+ if (fname)
+ SVN_ERR (svn_path_cstring_to_utf8 (fname_utf8, fname, pool));
+ else
+ *fname_utf8 = NULL;
+
+ return SVN_NO_ERROR;
+}
+
+
 svn_error_t *
 svn_stringbuf_from_aprfile (svn_stringbuf_t **result,
                             apr_file_t *file,
@@ -1001,23 +1023,10 @@
   apr_size_t len;
   apr_status_t apr_err;
   svn_stringbuf_t *res = svn_stringbuf_create("", pool);
- const char *fname;
   char buf[BUFSIZ];

   /* XXX: We should check the incoming data for being of type binary. */

- apr_err = apr_file_name_get (&fname, file);
- if (apr_err)
- return svn_error_create
- (apr_err, NULL,
- "svn_stringbuf_from_aprfile: failed to get filename");
-
- /* If the apr_file_t was opened with apr_file_open_std{in,out,err}, then we
- * wont get a filename for it. We assume that since we are reading, that in
- * this case we would only ever be using stdin. */
- if (NULL == fname)
- fname = "stdin";
-
   /* apr_file_read will not return data and eof in the same call. So this loop
    * is safe from missing read data. */
   len = sizeof(buf);
@@ -1033,9 +1042,14 @@
   if (!APR_STATUS_IS_EOF(apr_err))
     {
       const char *fname_utf8;
+ SVN_ERR (file_name_get (&fname_utf8, file, pool));

- SVN_ERR (svn_path_cstring_to_utf8 (&fname_utf8, fname, pool));
-
+ /* If the apr_file_t was opened with apr_file_open_std{in,out,err}, then
+ * we won't get a filename for it. We assume that since we are reading,
+ * that in this case we would only ever be using stdin. */
+ if (NULL == fname_utf8)
+ fname_utf8 = "(stdin)";
+
       return svn_error_createf
         (apr_err, NULL,
          "svn_stringbuf_from_aprfile: EOF not seen for '%s'", fname_utf8);
@@ -1706,6 +1720,29 @@

 svn_error_t *
+svn_io_file_close (apr_file_t *file, apr_pool_t *pool)
+{
+ apr_status_t status;
+
+ 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?)";
+
+ 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_stat (apr_finfo_t *finfo, const char *fname,
              apr_int32_t wanted, apr_pool_t *pool)
 {

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 6 05:10:21 2003

This is an archived mail posted to the Subversion Dev mailing list.