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

Re: svn commit: r924360 - in /subversion/trunk/subversion: include/svn_io.h libsvn_subr/io.c

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Wed, 17 Mar 2010 17:23:19 +0000

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
Received on 2010-03-17 18:23:54 CET

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