Re: [PATCH] issue #897: libsvn_client

Great! I'd happily apply the wrapper to the occurrances I find when sifting
through the libraries.

bye,

Erik.

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

-- 
NEU FÜR ALLE - GMX MediaCenter - für Fotos, Musik, Dateien...
Fotoalbum, File Sharing, MMS, Multimedia-Gruß, GMX FotoService
Jetzt kostenlos anmelden unter http://www.gmx.net
+++ 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