You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/03/17 17:40:51 UTC
svn commit: r924360 - in /subversion/trunk/subversion: include/svn_io.h
libsvn_subr/io.c
Author: rhuijben
Date: Wed Mar 17 16:40:51 2010
New Revision: 924360
URL: http://svn.apache.org/viewvc?rev=924360&view=rev
Log:
Make svn_io_file_mktemp() behave in the same way on all operating
systems.
* subversion/include/svn_io.h
(svn_io_file_mktemp): Make templ const.
* subversion/libsvn_subr/io.c
(svn_io_file_mktemp):
Update prototype. Use svn_path_cstring_from_utf8(), which makes a
local copy on all operating systems, to make sure apr never tries to
overwrite read only memory on systems with a utf-8 filesystem api in
apr (such as Mac/OS X and Windows).
Modified:
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_subr/io.c
Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=924360&r1=924359&r2=924360&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Wed Mar 17 16:40:51 2010
@@ -1904,7 +1904,7 @@ svn_io_write_version_file(const char *pa
* @since New in 1.7. */
svn_error_t *
svn_io_file_mktemp(apr_file_t **new_file,
- char *templ,
+ const char *templ,
apr_int32_t flags,
apr_pool_t *pool);
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=924360&r1=924359&r2=924360&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Wed Mar 17 16:40:51 2010
@@ -3525,13 +3525,13 @@ svn_io_files_contents_same_p(svn_boolean
/* Wrapper for apr_file_mktemp(). */
svn_error_t *
-svn_io_file_mktemp(apr_file_t **new_file, char *templ,
+svn_io_file_mktemp(apr_file_t **new_file, const char *templ,
apr_int32_t flags, apr_pool_t *pool)
{
const char *templ_apr;
apr_status_t status;
- SVN_ERR(cstring_from_utf8(&templ_apr, templ, pool));
+ SVN_ERR(svn_path_cstring_from_utf8(&templ_apr, templ, pool));
/* ### I don't want to copy the template string again just to
* make it writable... so cast away const.
Re: svn commit: r924360 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/io.c
Posted by Julian Foad <ju...@wandisco.com>.
Stefan Sperling wrote:
> > * subversion/libsvn_subr/io.c
> > (svn_io_file_mktemp):
> > Update prototype. Use svn_path_cstring_from_utf8(), which makes a
> > local copy on all operating systems, to make sure apr never tries to
> > overwrite read only memory on systems with a utf-8 filesystem api in
> > apr (such as Mac/OS X and Windows).
>
> It would be nice to have the last sentence from the log message
> as a comment in the code also. It's not obvious that we depend
> on this side-effect of svn_path_cstring_from_utf8() otherwise.
At the same time, we can change the local variable to "char *templ_apr"
to reflect that we know the memory will be writable, and cast its
address to "const char **" in the first call instead of the (slightly
nastier) present method of casting away const in the second call.
- Julian
> > svn_error_t *
> > -svn_io_file_mktemp(apr_file_t **new_file, char *templ,
> > +svn_io_file_mktemp(apr_file_t **new_file, const char *templ,
> > apr_int32_t flags, apr_pool_t *pool)
> > {
> > const char *templ_apr;
> > apr_status_t status;
> >
> > - SVN_ERR(cstring_from_utf8(&templ_apr, templ, pool));
> > + SVN_ERR(svn_path_cstring_from_utf8(&templ_apr, templ, pool));
> >
> > /* ### I don't want to copy the template string again just to
> > * make it writable... so cast away const.
> >
[[[
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 924197)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -3528,15 +3528,12 @@
svn_io_file_mktemp(apr_file_t **new_file, char *templ,
apr_int32_t flags, apr_pool_t *pool)
{
- const char *templ_apr;
+ char *templ_apr;
apr_status_t status;
- SVN_ERR(svn_path_cstring_from_utf8(&templ_apr, templ, pool));
+ SVN_ERR(svn_path_cstring_from_utf8((const char **)&templ_apr, templ, pool));
- /* ### I don't want to copy the template string again just to
- * make it writable... so cast away const.
- * Should the UTF-8 conversion functions really be returning const??? */
- status = apr_file_mktemp(new_file, (char *)templ_apr, flags, pool);
+ status = apr_file_mktemp(new_file, templ_apr, flags, pool);
if (status)
return svn_error_wrap_apr(status, _("Can't create temporary file from "
]]]
- Julian
Re: svn commit: r924360 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/io.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Mar 17, 2010 at 04:40:51PM -0000, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Wed Mar 17 16:40:51 2010
> New Revision: 924360
>
> URL: http://svn.apache.org/viewvc?rev=924360&view=rev
> Log:
> Make svn_io_file_mktemp() behave in the same way on all operating
> systems.
>
> * subversion/include/svn_io.h
> (svn_io_file_mktemp): Make templ const.
>
> * subversion/libsvn_subr/io.c
> (svn_io_file_mktemp):
> Update prototype. Use svn_path_cstring_from_utf8(), which makes a
> local copy on all operating systems, to make sure apr never tries to
> overwrite read only memory on systems with a utf-8 filesystem api in
> apr (such as Mac/OS X and Windows).
It would be nice to have the last sentence from the log message
as a comment in the code also. It's not obvious that we depend
on this side-effect of svn_path_cstring_from_utf8() otherwise.
Thanks,
Stefan
>
> Modified:
> subversion/trunk/subversion/include/svn_io.h
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/include/svn_io.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=924360&r1=924359&r2=924360&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_io.h (original)
> +++ subversion/trunk/subversion/include/svn_io.h Wed Mar 17 16:40:51 2010
> @@ -1904,7 +1904,7 @@ svn_io_write_version_file(const char *pa
> * @since New in 1.7. */
> svn_error_t *
> svn_io_file_mktemp(apr_file_t **new_file,
> - char *templ,
> + const char *templ,
> apr_int32_t flags,
> apr_pool_t *pool);
>
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=924360&r1=924359&r2=924360&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Wed Mar 17 16:40:51 2010
> @@ -3525,13 +3525,13 @@ svn_io_files_contents_same_p(svn_boolean
>
> /* Wrapper for apr_file_mktemp(). */
> svn_error_t *
> -svn_io_file_mktemp(apr_file_t **new_file, char *templ,
> +svn_io_file_mktemp(apr_file_t **new_file, const char *templ,
> apr_int32_t flags, apr_pool_t *pool)
> {
> const char *templ_apr;
> apr_status_t status;
>
> - SVN_ERR(cstring_from_utf8(&templ_apr, templ, pool));
> + SVN_ERR(svn_path_cstring_from_utf8(&templ_apr, templ, pool));
>
> /* ### I don't want to copy the template string again just to
> * make it writable... so cast away const.
>
--
printf("Eh???/n");