You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/06/22 18:18:34 UTC
svn commit: r956941 - /subversion/trunk/subversion/libsvn_subr/io.c
Author: julianfoad
Date: Tue Jun 22 16:18:34 2010
New Revision: 956941
URL: http://svn.apache.org/viewvc?rev=956941&view=rev
Log:
Update doc strings and parameter names to distinguish internal UTF-8
encoding from APR path encoding.
* subversion/libsvn_subr/io.c
(file_open): Rename the 'fname' parameter to 'fname_apr' and mention it in
the doc string.
(temp_file_cleanup_s): Rename the 'name' member to 'fname_apr' and
document it.
(temp_file_plain_cleanup_handler, svn_io_open_uniquely_named,
svn_io_open_unique_file3): Adjust all uses.
(file_perms_set, svn_io_file_name_get): Mention that filename parameters
are in UTF-8, as these are otherwise basically wrappers around APR.
Modified:
subversion/trunk/subversion/libsvn_subr/io.c
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=956941&r1=956940&r2=956941&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Tue Jun 22 16:18:34 2010
@@ -243,20 +243,20 @@ io_check_path(const char *path,
}
-/* Wrapper for apr_file_open(). */
+/* Wrapper for apr_file_open(), taking an APR-encoded filename. */
static apr_status_t
file_open(apr_file_t **f,
- const char *fname,
+ const char *fname_apr,
apr_int32_t flag,
apr_fileperms_t perm,
svn_boolean_t retry_on_failure,
apr_pool_t *pool)
{
- apr_status_t status = apr_file_open(f, fname, flag, perm, pool);
+ apr_status_t status = apr_file_open(f, fname_apr, flag, perm, pool);
if (retry_on_failure)
{
- WIN32_RETRY_LOOP(status, apr_file_open(f, fname, flag, perm, pool));
+ WIN32_RETRY_LOOP(status, apr_file_open(f, fname_apr, flag, perm, pool));
}
return status;
}
@@ -292,7 +292,9 @@ svn_io_check_special_path(const char *pa
struct temp_file_cleanup_s
{
apr_pool_t *pool;
- const char *name;
+ /* The (APR-encoded) full path of the file to be removed, or NULL if
+ * nothing to do. */
+ const char *fname_apr;
};
@@ -302,10 +304,10 @@ temp_file_plain_cleanup_handler(void *ba
struct temp_file_cleanup_s *b = baton;
apr_status_t apr_err = APR_SUCCESS;
- if (b->name)
+ if (b->fname_apr)
{
- apr_err = apr_file_remove(b->name, b->pool);
- WIN32_RETRY_LOOP(apr_err, apr_file_remove(b->name, b->pool));
+ apr_err = apr_file_remove(b->fname_apr, b->pool);
+ WIN32_RETRY_LOOP(apr_err, apr_file_remove(b->fname_apr, b->pool));
}
return apr_err;
@@ -354,7 +356,7 @@ svn_io_open_uniquely_named(apr_file_t **
baton = apr_palloc(result_pool, sizeof(*baton));
baton->pool = result_pool;
- baton->name = NULL;
+ baton->fname_apr = NULL;
/* Because cleanups are run LIFO, we need to make sure to register
our cleanup before the apr_file_close cleanup:
@@ -447,7 +449,7 @@ svn_io_open_uniquely_named(apr_file_t **
else
{
if (delete_when == svn_io_file_del_on_pool_cleanup)
- baton->name = apr_pstrdup(result_pool, unique_name_apr);
+ baton->fname_apr = apr_pstrdup(result_pool, unique_name_apr);
if (file)
*file = try_file;
@@ -786,7 +788,7 @@ svn_io_copy_file(const char *src,
return svn_error_return(svn_io_file_rename(dst_tmp, dst, pool));
}
-/* Wrapper for apr_file_perms_set(). */
+/* Wrapper for apr_file_perms_set(), taking a UTF8-encoded filename. */
static svn_error_t *
file_perms_set(const char *fname, apr_fileperms_t perms,
apr_pool_t *pool)
@@ -3665,7 +3667,7 @@ temp_file_create(apr_file_t **new_file,
#endif
}
-/* Wrapper for apr_file_name_get(). */
+/* Wrapper for apr_file_name_get(), passing out a UTF8-encoded filename. */
svn_error_t *
svn_io_file_name_get(const char **filename,
apr_file_t *file,
@@ -3718,7 +3720,7 @@ svn_io_open_unique_file3(apr_file_t **fi
case svn_io_file_del_on_pool_cleanup:
baton = apr_palloc(result_pool, sizeof(*baton));
baton->pool = result_pool;
- baton->name = NULL;
+ baton->fname_apr = NULL;
/* Because cleanups are run LIFO, we need to make sure to register
our cleanup before the apr_file_close cleanup:
@@ -3763,7 +3765,7 @@ svn_io_open_unique_file3(apr_file_t **fi
*unique_path = tempname; /* Was allocated in result_pool */
if (baton)
- SVN_ERR(cstring_from_utf8(&baton->name, tempname, result_pool));
+ SVN_ERR(cstring_from_utf8(&baton->fname_apr, tempname, result_pool));
return SVN_NO_ERROR;
}
Re: svn commit: r956941 -
/subversion/trunk/subversion/libsvn_subr/io.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-06-22 at 23:18 +0300, Daniel Shahaf wrote:
> julianfoad@apache.org wrote on Tue, 22 Jun 2010 at 19:18 -0000:
> > @@ -3665,7 +3667,7 @@ temp_file_create(apr_file_t **new_file,
> > #endif
> > }
> >
> > -/* Wrapper for apr_file_name_get(). */
> > +/* Wrapper for apr_file_name_get(), passing out a UTF8-encoded filename. */
> > svn_error_t *
> > svn_io_file_name_get(const char **filename,
> > apr_file_t *file,
>
> Doesn't this also belong in the public docs of this function?
Yes, thanks. Will commit when svn.a.o comes back on line.
(At first, I thought "No, we don't document when strings are UTF-8,
because that's normal", but then I looked in svn_io.h and saw that we do
document it on the "wrapper" functions because it's not obvious.)
- Julian
> [[[
> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revision 956596)
> +++ subversion/include/svn_io.h (working copy)
> @@ -1921,7 +1921,8 @@
> int version,
> apr_pool_t *pool);
>
> -/** Wrapper for apr_file_name_get().
> +/** Wrapper for apr_file_name_get(). The encoding of @a *filename
> + * is UTF-8.
> *
> * @since New in 1.7. */
> svn_error_t *
> ]]]
>
> While here, thanks for continually ensuring that we have proper docs
> for our internal functions.
>
> Daniel
Re: svn commit: r956941 -
/subversion/trunk/subversion/libsvn_subr/io.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-06-22 at 23:18 +0300, Daniel Shahaf wrote:
> julianfoad@apache.org wrote on Tue, 22 Jun 2010 at 19:18 -0000:
> > @@ -3665,7 +3667,7 @@ temp_file_create(apr_file_t **new_file,
> > #endif
> > }
> >
> > -/* Wrapper for apr_file_name_get(). */
> > +/* Wrapper for apr_file_name_get(), passing out a UTF8-encoded filename. */
> > svn_error_t *
> > svn_io_file_name_get(const char **filename,
> > apr_file_t *file,
>
> Doesn't this also belong in the public docs of this function?
Yes, thanks. Will commit when svn.a.o comes back on line.
(At first, I thought "No, we don't document when strings are UTF-8,
because that's normal", but then I looked in svn_io.h and saw that we do
document it on the "wrapper" functions because it's not obvious.)
- Julian
> [[[
> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revision 956596)
> +++ subversion/include/svn_io.h (working copy)
> @@ -1921,7 +1921,8 @@
> int version,
> apr_pool_t *pool);
>
> -/** Wrapper for apr_file_name_get().
> +/** Wrapper for apr_file_name_get(). The encoding of @a *filename
> + * is UTF-8.
> *
> * @since New in 1.7. */
> svn_error_t *
> ]]]
>
> While here, thanks for continually ensuring that we have proper docs
> for our internal functions.
>
> Daniel
Re: svn commit: r956941 -
/subversion/trunk/subversion/libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
julianfoad@apache.org wrote on Tue, 22 Jun 2010 at 19:18 -0000:
> @@ -3665,7 +3667,7 @@ temp_file_create(apr_file_t **new_file,
> #endif
> }
>
> -/* Wrapper for apr_file_name_get(). */
> +/* Wrapper for apr_file_name_get(), passing out a UTF8-encoded filename. */
> svn_error_t *
> svn_io_file_name_get(const char **filename,
> apr_file_t *file,
Doesn't this also belong in the public docs of this function?
[[[
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 956596)
+++ subversion/include/svn_io.h (working copy)
@@ -1921,7 +1921,8 @@
int version,
apr_pool_t *pool);
-/** Wrapper for apr_file_name_get().
+/** Wrapper for apr_file_name_get(). The encoding of @a *filename
+ * is UTF-8.
*
* @since New in 1.7. */
svn_error_t *
]]]
While here, thanks for continually ensuring that we have proper docs
for our internal functions.
Daniel
Re: svn commit: r956941 -
/subversion/trunk/subversion/libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
julianfoad@apache.org wrote on Tue, 22 Jun 2010 at 19:18 -0000:
> @@ -3665,7 +3667,7 @@ temp_file_create(apr_file_t **new_file,
> #endif
> }
>
> -/* Wrapper for apr_file_name_get(). */
> +/* Wrapper for apr_file_name_get(), passing out a UTF8-encoded filename. */
> svn_error_t *
> svn_io_file_name_get(const char **filename,
> apr_file_t *file,
Doesn't this also belong in the public docs of this function?
[[[
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 956596)
+++ subversion/include/svn_io.h (working copy)
@@ -1921,7 +1921,8 @@
int version,
apr_pool_t *pool);
-/** Wrapper for apr_file_name_get().
+/** Wrapper for apr_file_name_get(). The encoding of @a *filename
+ * is UTF-8.
*
* @since New in 1.7. */
svn_error_t *
]]]
While here, thanks for continually ensuring that we have proper docs
for our internal functions.
Daniel