You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2011/05/17 12:55:51 UTC
svn commit: r1104124 - in /subversion/trunk/subversion: include/svn_io.h
libsvn_subr/deprecated.c libsvn_subr/io.c
Author: danielsh
Date: Tue May 17 10:55:51 2011
New Revision: 1104124
URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
Log:
Revv the svn_io_file_create() API to take non-NUL-terminated strings.
* subversion/include/svn_io.h
(svn_io_file_create2): New.
(svn_io_file_create): Deprecate.
* subversion/libsvn_subr/io.c
(svn_io_file_create2): Renamed from svn_io_file_create().
* subversion/libsvn_subr/deprecated.c
(svn_io_file_create): New wrapper.
Modified:
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_subr/deprecated.c
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=1104124&r1=1104123&r2=1104124&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
@@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
/** Create file at utf8-encoded @a file with contents @a contents.
* @a file must not already exist.
* Use @a pool for memory allocations.
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_io_file_create2(const char *file,
+ const svn_string_t *contents,
+ apr_pool_t *scratch_pool);
+
+/** Like svn_io_file_create2(), but with a C string instead
+ * of an #svn_string_t.
+ *
+ * @deprecated Provided for backward compatibility with the 1.6 API.
*/
+SVN_DEPRECATED
svn_error_t *
svn_io_file_create(const char *file,
const char *contents,
Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
@@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
/*** From io.c ***/
svn_error_t *
+svn_io_file_create(const char *file,
+ const char *contents,
+ apr_pool_t *pool)
+{
+ const svn_string_t *contents_string;
+
+ contents_string = (contents ? svn_string_create(contents, pool) : NULL);
+ return svn_io_file_create2(file, contents_string, pool);
+}
+
+svn_error_t *
svn_io_open_unique_file2(apr_file_t **file,
const char **temp_path,
const char *path,
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
@@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
return SVN_NO_ERROR;
}
-svn_error_t *svn_io_file_create(const char *file,
- const char *contents,
- apr_pool_t *pool)
+svn_error_t *svn_io_file_create2(const char *file,
+ const svn_string_t *contents,
+ apr_pool_t *pool)
{
apr_file_t *f;
apr_size_t written;
@@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
(APR_WRITE | APR_CREATE | APR_EXCL),
APR_OS_DEFAULT,
pool));
- if (contents && *contents)
- err = svn_io_file_write_full(f, contents, strlen(contents),
+ if (contents && contents->len)
+ err = svn_io_file_write_full(f, contents->data, contents->len,
&written, pool);
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
If there isn't a pending need, delete it. No point in caring forward
API baggage if we don't need to.
-Hyrum
On Fri, May 27, 2011 at 3:40 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Yes, I disable deprecation warnings in my build. (and I indeed didn't do
> the "update existing callers" dance this time)
>
> I'm happy to go and remove the deprecation decorators from the old API,
> if people prefer that. I'm not sure what to do with the new API ---
> it's unused currently (other than the call from deprecated.c), so I'm
> not sure it should even remain as a public API.
>
> Greg Stein wrote on Fri, May 27, 2011 at 05:51:16 -0400:
>> There are a lot of callers for the old API.
>>
>> I'm kind of with Hyrum here. This new API isn't so much a
>> next-generation, as it is an API serving different needs. Or more
>> specifically: the old API of "provide a 'const char *'" is still
>> entirely valid.
>>
>> There are a bunch of warnings in the build for the (deprecated) calls
>> to the old API. By any chance, do you disable those in your build? I
>> know that you've been working hard on watching errors, so I'm
>> surprised you didn't see those. ?
>>
>> Cheers,
>> -g
>>
>> On Tue, May 17, 2011 at 08:45, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>> > Don't know; I haven't examined existing callers.
>> >
>> > It also turns out that I may not need the new (revv'd) API at all ---
>> > went a different way in the patch I'm working on --- so I may, after
>> > all, not add a user of the new API today (as I'd planned to).
>> >
>> > Hyrum K Wright wrote on Tue, May 17, 2011 at 11:20:59 +0000:
>> >> Are there still valid use cases for the normal cstring case? If so,
>> >> we shouldn't be deprecating the old API, but rather adding a distinct
>> >> API.
>> >>
>> >> -Hyrum
>> >>
>> >> On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
>> >> > Author: danielsh
>> >> > Date: Tue May 17 10:55:51 2011
>> >> > New Revision: 1104124
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
>> >> > Log:
>> >> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
>> >> >
>> >> > * subversion/include/svn_io.h
>> >> > (svn_io_file_create2): New.
>> >> > (svn_io_file_create): Deprecate.
>> >> >
>> >> > * subversion/libsvn_subr/io.c
>> >> > (svn_io_file_create2): Renamed from svn_io_file_create().
>> >> >
>> >> > * subversion/libsvn_subr/deprecated.c
>> >> > (svn_io_file_create): New wrapper.
>> >> >
>> >> > Modified:
>> >> > subversion/trunk/subversion/include/svn_io.h
>> >> > subversion/trunk/subversion/libsvn_subr/deprecated.c
>> >> > 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=1104124&r1=1104123&r2=1104124&view=diff
>> >> > ==============================================================================
>> >> > --- subversion/trunk/subversion/include/svn_io.h (original)
>> >> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
>> >> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
>> >> > /** Create file at utf8-encoded @a file with contents @a contents.
>> >> > * @a file must not already exist.
>> >> > * Use @a pool for memory allocations.
>> >> > + *
>> >> > + * @since New in 1.7.
>> >> > + */
>> >> > +svn_error_t *
>> >> > +svn_io_file_create2(const char *file,
>> >> > + const svn_string_t *contents,
>> >> > + apr_pool_t *scratch_pool);
>> >> > +
>> >> > +/** Like svn_io_file_create2(), but with a C string instead
>> >> > + * of an #svn_string_t.
>> >> > + *
>> >> > + * @deprecated Provided for backward compatibility with the 1.6 API.
>> >> > */
>> >> > +SVN_DEPRECATED
>> >> > svn_error_t *
>> >> > svn_io_file_create(const char *file,
>> >> > const char *contents,
>> >> >
>> >> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
>> >> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
>> >> > ==============================================================================
>> >> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
>> >> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
>> >> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>> >> >
>> >> > /*** From io.c ***/
>> >> > svn_error_t *
>> >> > +svn_io_file_create(const char *file,
>> >> > + const char *contents,
>> >> > + apr_pool_t *pool)
>> >> > +{
>> >> > + const svn_string_t *contents_string;
>> >> > +
>> >> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
>> >> > + return svn_io_file_create2(file, contents_string, pool);
>> >> > +}
>> >> > +
>> >> > +svn_error_t *
>> >> > svn_io_open_unique_file2(apr_file_t **file,
>> >> > const char **temp_path,
>> >> > const char *path,
>> >> >
>> >> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
>> >> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
>> >> > ==============================================================================
>> >> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
>> >> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
>> >> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
>> >> > return SVN_NO_ERROR;
>> >> > }
>> >> >
>> >> > -svn_error_t *svn_io_file_create(const char *file,
>> >> > - const char *contents,
>> >> > - apr_pool_t *pool)
>> >> > +svn_error_t *svn_io_file_create2(const char *file,
>> >> > + const svn_string_t *contents,
>> >> > + apr_pool_t *pool)
>> >> > {
>> >> > apr_file_t *f;
>> >> > apr_size_t written;
>> >> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
>> >> > (APR_WRITE | APR_CREATE | APR_EXCL),
>> >> > APR_OS_DEFAULT,
>> >> > pool));
>> >> > - if (contents && *contents)
>> >> > - err = svn_io_file_write_full(f, contents, strlen(contents),
>> >> > + if (contents && contents->len)
>> >> > + err = svn_io_file_write_full(f, contents->data, contents->len,
>> >> > &written, pool);
>> >> >
>> >> >
>> >> >
>> >> >
>> >> >
>> >
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Yes, I disable deprecation warnings in my build. (and I indeed didn't do
the "update existing callers" dance this time)
I'm happy to go and remove the deprecation decorators from the old API,
if people prefer that. I'm not sure what to do with the new API ---
it's unused currently (other than the call from deprecated.c), so I'm
not sure it should even remain as a public API.
Greg Stein wrote on Fri, May 27, 2011 at 05:51:16 -0400:
> There are a lot of callers for the old API.
>
> I'm kind of with Hyrum here. This new API isn't so much a
> next-generation, as it is an API serving different needs. Or more
> specifically: the old API of "provide a 'const char *'" is still
> entirely valid.
>
> There are a bunch of warnings in the build for the (deprecated) calls
> to the old API. By any chance, do you disable those in your build? I
> know that you've been working hard on watching errors, so I'm
> surprised you didn't see those. ?
>
> Cheers,
> -g
>
> On Tue, May 17, 2011 at 08:45, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > Don't know; I haven't examined existing callers.
> >
> > It also turns out that I may not need the new (revv'd) API at all ---
> > went a different way in the patch I'm working on --- so I may, after
> > all, not add a user of the new API today (as I'd planned to).
> >
> > Hyrum K Wright wrote on Tue, May 17, 2011 at 11:20:59 +0000:
> >> Are there still valid use cases for the normal cstring case? If so,
> >> we shouldn't be deprecating the old API, but rather adding a distinct
> >> API.
> >>
> >> -Hyrum
> >>
> >> On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
> >> > Author: danielsh
> >> > Date: Tue May 17 10:55:51 2011
> >> > New Revision: 1104124
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> >> > Log:
> >> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
> >> >
> >> > * subversion/include/svn_io.h
> >> > (svn_io_file_create2): New.
> >> > (svn_io_file_create): Deprecate.
> >> >
> >> > * subversion/libsvn_subr/io.c
> >> > (svn_io_file_create2): Renamed from svn_io_file_create().
> >> >
> >> > * subversion/libsvn_subr/deprecated.c
> >> > (svn_io_file_create): New wrapper.
> >> >
> >> > Modified:
> >> > subversion/trunk/subversion/include/svn_io.h
> >> > subversion/trunk/subversion/libsvn_subr/deprecated.c
> >> > 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=1104124&r1=1104123&r2=1104124&view=diff
> >> > ==============================================================================
> >> > --- subversion/trunk/subversion/include/svn_io.h (original)
> >> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> >> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> >> > /** Create file at utf8-encoded @a file with contents @a contents.
> >> > * @a file must not already exist.
> >> > * Use @a pool for memory allocations.
> >> > + *
> >> > + * @since New in 1.7.
> >> > + */
> >> > +svn_error_t *
> >> > +svn_io_file_create2(const char *file,
> >> > + const svn_string_t *contents,
> >> > + apr_pool_t *scratch_pool);
> >> > +
> >> > +/** Like svn_io_file_create2(), but with a C string instead
> >> > + * of an #svn_string_t.
> >> > + *
> >> > + * @deprecated Provided for backward compatibility with the 1.6 API.
> >> > */
> >> > +SVN_DEPRECATED
> >> > svn_error_t *
> >> > svn_io_file_create(const char *file,
> >> > const char *contents,
> >> >
> >> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> >> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> >> > ==============================================================================
> >> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> >> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> >> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
> >> >
> >> > /*** From io.c ***/
> >> > svn_error_t *
> >> > +svn_io_file_create(const char *file,
> >> > + const char *contents,
> >> > + apr_pool_t *pool)
> >> > +{
> >> > + const svn_string_t *contents_string;
> >> > +
> >> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> >> > + return svn_io_file_create2(file, contents_string, pool);
> >> > +}
> >> > +
> >> > +svn_error_t *
> >> > svn_io_open_unique_file2(apr_file_t **file,
> >> > const char **temp_path,
> >> > const char *path,
> >> >
> >> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
> >> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> >> > ==============================================================================
> >> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> >> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> >> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> >> > return SVN_NO_ERROR;
> >> > }
> >> >
> >> > -svn_error_t *svn_io_file_create(const char *file,
> >> > - const char *contents,
> >> > - apr_pool_t *pool)
> >> > +svn_error_t *svn_io_file_create2(const char *file,
> >> > + const svn_string_t *contents,
> >> > + apr_pool_t *pool)
> >> > {
> >> > apr_file_t *f;
> >> > apr_size_t written;
> >> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> >> > (APR_WRITE | APR_CREATE | APR_EXCL),
> >> > APR_OS_DEFAULT,
> >> > pool));
> >> > - if (contents && *contents)
> >> > - err = svn_io_file_write_full(f, contents, strlen(contents),
> >> > + if (contents && contents->len)
> >> > + err = svn_io_file_write_full(f, contents->data, contents->len,
> >> > &written, pool);
> >> >
> >> >
> >> >
> >> >
> >> >
> >
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Greg Stein <gs...@gmail.com>.
There are a lot of callers for the old API.
I'm kind of with Hyrum here. This new API isn't so much a
next-generation, as it is an API serving different needs. Or more
specifically: the old API of "provide a 'const char *'" is still
entirely valid.
There are a bunch of warnings in the build for the (deprecated) calls
to the old API. By any chance, do you disable those in your build? I
know that you've been working hard on watching errors, so I'm
surprised you didn't see those. ?
Cheers,
-g
On Tue, May 17, 2011 at 08:45, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Don't know; I haven't examined existing callers.
>
> It also turns out that I may not need the new (revv'd) API at all ---
> went a different way in the patch I'm working on --- so I may, after
> all, not add a user of the new API today (as I'd planned to).
>
> Hyrum K Wright wrote on Tue, May 17, 2011 at 11:20:59 +0000:
>> Are there still valid use cases for the normal cstring case? If so,
>> we shouldn't be deprecating the old API, but rather adding a distinct
>> API.
>>
>> -Hyrum
>>
>> On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
>> > Author: danielsh
>> > Date: Tue May 17 10:55:51 2011
>> > New Revision: 1104124
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
>> > Log:
>> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
>> >
>> > * subversion/include/svn_io.h
>> > (svn_io_file_create2): New.
>> > (svn_io_file_create): Deprecate.
>> >
>> > * subversion/libsvn_subr/io.c
>> > (svn_io_file_create2): Renamed from svn_io_file_create().
>> >
>> > * subversion/libsvn_subr/deprecated.c
>> > (svn_io_file_create): New wrapper.
>> >
>> > Modified:
>> > subversion/trunk/subversion/include/svn_io.h
>> > subversion/trunk/subversion/libsvn_subr/deprecated.c
>> > 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=1104124&r1=1104123&r2=1104124&view=diff
>> > ==============================================================================
>> > --- subversion/trunk/subversion/include/svn_io.h (original)
>> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
>> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
>> > /** Create file at utf8-encoded @a file with contents @a contents.
>> > * @a file must not already exist.
>> > * Use @a pool for memory allocations.
>> > + *
>> > + * @since New in 1.7.
>> > + */
>> > +svn_error_t *
>> > +svn_io_file_create2(const char *file,
>> > + const svn_string_t *contents,
>> > + apr_pool_t *scratch_pool);
>> > +
>> > +/** Like svn_io_file_create2(), but with a C string instead
>> > + * of an #svn_string_t.
>> > + *
>> > + * @deprecated Provided for backward compatibility with the 1.6 API.
>> > */
>> > +SVN_DEPRECATED
>> > svn_error_t *
>> > svn_io_file_create(const char *file,
>> > const char *contents,
>> >
>> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
>> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
>> > ==============================================================================
>> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
>> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
>> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>> >
>> > /*** From io.c ***/
>> > svn_error_t *
>> > +svn_io_file_create(const char *file,
>> > + const char *contents,
>> > + apr_pool_t *pool)
>> > +{
>> > + const svn_string_t *contents_string;
>> > +
>> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
>> > + return svn_io_file_create2(file, contents_string, pool);
>> > +}
>> > +
>> > +svn_error_t *
>> > svn_io_open_unique_file2(apr_file_t **file,
>> > const char **temp_path,
>> > const char *path,
>> >
>> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
>> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
>> > ==============================================================================
>> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
>> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
>> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
>> > return SVN_NO_ERROR;
>> > }
>> >
>> > -svn_error_t *svn_io_file_create(const char *file,
>> > - const char *contents,
>> > - apr_pool_t *pool)
>> > +svn_error_t *svn_io_file_create2(const char *file,
>> > + const svn_string_t *contents,
>> > + apr_pool_t *pool)
>> > {
>> > apr_file_t *f;
>> > apr_size_t written;
>> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
>> > (APR_WRITE | APR_CREATE | APR_EXCL),
>> > APR_OS_DEFAULT,
>> > pool));
>> > - if (contents && *contents)
>> > - err = svn_io_file_write_full(f, contents, strlen(contents),
>> > + if (contents && contents->len)
>> > + err = svn_io_file_write_full(f, contents->data, contents->len,
>> > &written, pool);
>> >
>> >
>> >
>> >
>> >
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Don't know; I haven't examined existing callers.
It also turns out that I may not need the new (revv'd) API at all ---
went a different way in the patch I'm working on --- so I may, after
all, not add a user of the new API today (as I'd planned to).
Hyrum K Wright wrote on Tue, May 17, 2011 at 11:20:59 +0000:
> Are there still valid use cases for the normal cstring case? If so,
> we shouldn't be deprecating the old API, but rather adding a distinct
> API.
>
> -Hyrum
>
> On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
> > Author: danielsh
> > Date: Tue May 17 10:55:51 2011
> > New Revision: 1104124
> >
> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> > Log:
> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
> >
> > * subversion/include/svn_io.h
> > (svn_io_file_create2): New.
> > (svn_io_file_create): Deprecate.
> >
> > * subversion/libsvn_subr/io.c
> > (svn_io_file_create2): Renamed from svn_io_file_create().
> >
> > * subversion/libsvn_subr/deprecated.c
> > (svn_io_file_create): New wrapper.
> >
> > Modified:
> > subversion/trunk/subversion/include/svn_io.h
> > subversion/trunk/subversion/libsvn_subr/deprecated.c
> > 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=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/include/svn_io.h (original)
> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> > /** Create file at utf8-encoded @a file with contents @a contents.
> > * @a file must not already exist.
> > * Use @a pool for memory allocations.
> > + *
> > + * @since New in 1.7.
> > + */
> > +svn_error_t *
> > +svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *scratch_pool);
> > +
> > +/** Like svn_io_file_create2(), but with a C string instead
> > + * of an #svn_string_t.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.6 API.
> > */
> > +SVN_DEPRECATED
> > svn_error_t *
> > svn_io_file_create(const char *file,
> > const char *contents,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
> >
> > /*** From io.c ***/
> > svn_error_t *
> > +svn_io_file_create(const char *file,
> > + const char *contents,
> > + apr_pool_t *pool)
> > +{
> > + const svn_string_t *contents_string;
> > +
> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> > + return svn_io_file_create2(file, contents_string, pool);
> > +}
> > +
> > +svn_error_t *
> > svn_io_open_unique_file2(apr_file_t **file,
> > const char **temp_path,
> > const char *path,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> > return SVN_NO_ERROR;
> > }
> >
> > -svn_error_t *svn_io_file_create(const char *file,
> > - const char *contents,
> > - apr_pool_t *pool)
> > +svn_error_t *svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *pool)
> > {
> > apr_file_t *f;
> > apr_size_t written;
> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> > (APR_WRITE | APR_CREATE | APR_EXCL),
> > APR_OS_DEFAULT,
> > pool));
> > - if (contents && *contents)
> > - err = svn_io_file_write_full(f, contents, strlen(contents),
> > + if (contents && contents->len)
> > + err = svn_io_file_write_full(f, contents->data, contents->len,
> > &written, pool);
> >
> >
> >
> >
> >
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Don't know; I haven't examined existing callers.
It also turns out that I may not need the new (revv'd) API at all ---
went a different way in the patch I'm working on --- so I may, after
all, not add a user of the new API today (as I'd planned to).
Hyrum K Wright wrote on Tue, May 17, 2011 at 11:20:59 +0000:
> Are there still valid use cases for the normal cstring case? If so,
> we shouldn't be deprecating the old API, but rather adding a distinct
> API.
>
> -Hyrum
>
> On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
> > Author: danielsh
> > Date: Tue May 17 10:55:51 2011
> > New Revision: 1104124
> >
> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> > Log:
> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
> >
> > * subversion/include/svn_io.h
> > (svn_io_file_create2): New.
> > (svn_io_file_create): Deprecate.
> >
> > * subversion/libsvn_subr/io.c
> > (svn_io_file_create2): Renamed from svn_io_file_create().
> >
> > * subversion/libsvn_subr/deprecated.c
> > (svn_io_file_create): New wrapper.
> >
> > Modified:
> > subversion/trunk/subversion/include/svn_io.h
> > subversion/trunk/subversion/libsvn_subr/deprecated.c
> > 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=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/include/svn_io.h (original)
> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> > /** Create file at utf8-encoded @a file with contents @a contents.
> > * @a file must not already exist.
> > * Use @a pool for memory allocations.
> > + *
> > + * @since New in 1.7.
> > + */
> > +svn_error_t *
> > +svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *scratch_pool);
> > +
> > +/** Like svn_io_file_create2(), but with a C string instead
> > + * of an #svn_string_t.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.6 API.
> > */
> > +SVN_DEPRECATED
> > svn_error_t *
> > svn_io_file_create(const char *file,
> > const char *contents,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
> >
> > /*** From io.c ***/
> > svn_error_t *
> > +svn_io_file_create(const char *file,
> > + const char *contents,
> > + apr_pool_t *pool)
> > +{
> > + const svn_string_t *contents_string;
> > +
> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> > + return svn_io_file_create2(file, contents_string, pool);
> > +}
> > +
> > +svn_error_t *
> > svn_io_open_unique_file2(apr_file_t **file,
> > const char **temp_path,
> > const char *path,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> > return SVN_NO_ERROR;
> > }
> >
> > -svn_error_t *svn_io_file_create(const char *file,
> > - const char *contents,
> > - apr_pool_t *pool)
> > +svn_error_t *svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *pool)
> > {
> > apr_file_t *f;
> > apr_size_t written;
> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> > (APR_WRITE | APR_CREATE | APR_EXCL),
> > APR_OS_DEFAULT,
> > pool));
> > - if (contents && *contents)
> > - err = svn_io_file_write_full(f, contents, strlen(contents),
> > + if (contents && contents->len)
> > + err = svn_io_file_write_full(f, contents->data, contents->len,
> > &written, pool);
> >
> >
> >
> >
> >
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
Are there still valid use cases for the normal cstring case? If so,
we shouldn't be deprecating the old API, but rather adding a distinct
API.
-Hyrum
On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
> Author: danielsh
> Date: Tue May 17 10:55:51 2011
> New Revision: 1104124
>
> URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> Log:
> Revv the svn_io_file_create() API to take non-NUL-terminated strings.
>
> * subversion/include/svn_io.h
> (svn_io_file_create2): New.
> (svn_io_file_create): Deprecate.
>
> * subversion/libsvn_subr/io.c
> (svn_io_file_create2): Renamed from svn_io_file_create().
>
> * subversion/libsvn_subr/deprecated.c
> (svn_io_file_create): New wrapper.
>
> Modified:
> subversion/trunk/subversion/include/svn_io.h
> subversion/trunk/subversion/libsvn_subr/deprecated.c
> 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=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_io.h (original)
> +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> /** Create file at utf8-encoded @a file with contents @a contents.
> * @a file must not already exist.
> * Use @a pool for memory allocations.
> + *
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *scratch_pool);
> +
> +/** Like svn_io_file_create2(), but with a C string instead
> + * of an #svn_string_t.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_io_file_create(const char *file,
> const char *contents,
>
> Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>
> /*** From io.c ***/
> svn_error_t *
> +svn_io_file_create(const char *file,
> + const char *contents,
> + apr_pool_t *pool)
> +{
> + const svn_string_t *contents_string;
> +
> + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> + return svn_io_file_create2(file, contents_string, pool);
> +}
> +
> +svn_error_t *
> svn_io_open_unique_file2(apr_file_t **file,
> const char **temp_path,
> const char *path,
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> return SVN_NO_ERROR;
> }
>
> -svn_error_t *svn_io_file_create(const char *file,
> - const char *contents,
> - apr_pool_t *pool)
> +svn_error_t *svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *pool)
> {
> apr_file_t *f;
> apr_size_t written;
> @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> (APR_WRITE | APR_CREATE | APR_EXCL),
> APR_OS_DEFAULT,
> pool));
> - if (contents && *contents)
> - err = svn_io_file_write_full(f, contents, strlen(contents),
> + if (contents && contents->len)
> + err = svn_io_file_write_full(f, contents->data, contents->len,
> &written, pool);
>
>
>
>
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
Are there still valid use cases for the normal cstring case? If so,
we shouldn't be deprecating the old API, but rather adding a distinct
API.
-Hyrum
On Tue, May 17, 2011 at 10:55 AM, <da...@apache.org> wrote:
> Author: danielsh
> Date: Tue May 17 10:55:51 2011
> New Revision: 1104124
>
> URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> Log:
> Revv the svn_io_file_create() API to take non-NUL-terminated strings.
>
> * subversion/include/svn_io.h
> (svn_io_file_create2): New.
> (svn_io_file_create): Deprecate.
>
> * subversion/libsvn_subr/io.c
> (svn_io_file_create2): Renamed from svn_io_file_create().
>
> * subversion/libsvn_subr/deprecated.c
> (svn_io_file_create): New wrapper.
>
> Modified:
> subversion/trunk/subversion/include/svn_io.h
> subversion/trunk/subversion/libsvn_subr/deprecated.c
> 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=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_io.h (original)
> +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> /** Create file at utf8-encoded @a file with contents @a contents.
> * @a file must not already exist.
> * Use @a pool for memory allocations.
> + *
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *scratch_pool);
> +
> +/** Like svn_io_file_create2(), but with a C string instead
> + * of an #svn_string_t.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_io_file_create(const char *file,
> const char *contents,
>
> Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>
> /*** From io.c ***/
> svn_error_t *
> +svn_io_file_create(const char *file,
> + const char *contents,
> + apr_pool_t *pool)
> +{
> + const svn_string_t *contents_string;
> +
> + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> + return svn_io_file_create2(file, contents_string, pool);
> +}
> +
> +svn_error_t *
> svn_io_open_unique_file2(apr_file_t **file,
> const char **temp_path,
> const char *path,
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> return SVN_NO_ERROR;
> }
>
> -svn_error_t *svn_io_file_create(const char *file,
> - const char *contents,
> - apr_pool_t *pool)
> +svn_error_t *svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *pool)
> {
> apr_file_t *f;
> apr_size_t written;
> @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> (APR_WRITE | APR_CREATE | APR_EXCL),
> APR_OS_DEFAULT,
> pool));
> - if (contents && *contents)
> - err = svn_io_file_write_full(f, contents, strlen(contents),
> + if (contents && contents->len)
> + err = svn_io_file_write_full(f, contents->data, contents->len,
> &written, pool);
>
>
>
>
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, May 17, 2011 at 12:00:25 +0100:
> Not directly related to your change, but I notice the impl. allows the
> passed-in string to be null if not wanted. Maybe the doc string should
> promise that,
/me nods
> otherwise why bother?
>
Because the existing code allowed NULLs, I made the new code accept
NULLs as well.
> - Julian
>
>
> On Tue, 2011-05-17 at 10:55 +0000, danielsh@apache.org wrote:
> > Author: danielsh
> > Date: Tue May 17 10:55:51 2011
> > New Revision: 1104124
> >
> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> > Log:
> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
> >
> > * subversion/include/svn_io.h
> > (svn_io_file_create2): New.
> > (svn_io_file_create): Deprecate.
> >
> > * subversion/libsvn_subr/io.c
> > (svn_io_file_create2): Renamed from svn_io_file_create().
> >
> > * subversion/libsvn_subr/deprecated.c
> > (svn_io_file_create): New wrapper.
> >
> > Modified:
> > subversion/trunk/subversion/include/svn_io.h
> > subversion/trunk/subversion/libsvn_subr/deprecated.c
> > 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=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/include/svn_io.h (original)
> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> > /** Create file at utf8-encoded @a file with contents @a contents.
> > * @a file must not already exist.
> > * Use @a pool for memory allocations.
> > + *
> > + * @since New in 1.7.
> > + */
> > +svn_error_t *
> > +svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *scratch_pool);
> > +
> > +/** Like svn_io_file_create2(), but with a C string instead
> > + * of an #svn_string_t.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.6 API.
> > */
> > +SVN_DEPRECATED
> > svn_error_t *
> > svn_io_file_create(const char *file,
> > const char *contents,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
> >
> > /*** From io.c ***/
> > svn_error_t *
> > +svn_io_file_create(const char *file,
> > + const char *contents,
> > + apr_pool_t *pool)
> > +{
> > + const svn_string_t *contents_string;
> > +
> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> > + return svn_io_file_create2(file, contents_string, pool);
> > +}
> > +
> > +svn_error_t *
> > svn_io_open_unique_file2(apr_file_t **file,
> > const char **temp_path,
> > const char *path,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> > return SVN_NO_ERROR;
> > }
> >
> > -svn_error_t *svn_io_file_create(const char *file,
> > - const char *contents,
> > - apr_pool_t *pool)
> > +svn_error_t *svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *pool)
> > {
> > apr_file_t *f;
> > apr_size_t written;
> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> > (APR_WRITE | APR_CREATE | APR_EXCL),
> > APR_OS_DEFAULT,
> > pool));
> > - if (contents && *contents)
> > - err = svn_io_file_write_full(f, contents, strlen(contents),
> > + if (contents && contents->len)
> > + err = svn_io_file_write_full(f, contents->data, contents->len,
> > &written, pool);
> >
> >
> >
> >
>
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Tue, May 17, 2011 at 12:00:25 +0100:
> Not directly related to your change, but I notice the impl. allows the
> passed-in string to be null if not wanted. Maybe the doc string should
> promise that,
/me nods
> otherwise why bother?
>
Because the existing code allowed NULLs, I made the new code accept
NULLs as well.
> - Julian
>
>
> On Tue, 2011-05-17 at 10:55 +0000, danielsh@apache.org wrote:
> > Author: danielsh
> > Date: Tue May 17 10:55:51 2011
> > New Revision: 1104124
> >
> > URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> > Log:
> > Revv the svn_io_file_create() API to take non-NUL-terminated strings.
> >
> > * subversion/include/svn_io.h
> > (svn_io_file_create2): New.
> > (svn_io_file_create): Deprecate.
> >
> > * subversion/libsvn_subr/io.c
> > (svn_io_file_create2): Renamed from svn_io_file_create().
> >
> > * subversion/libsvn_subr/deprecated.c
> > (svn_io_file_create): New wrapper.
> >
> > Modified:
> > subversion/trunk/subversion/include/svn_io.h
> > subversion/trunk/subversion/libsvn_subr/deprecated.c
> > 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=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/include/svn_io.h (original)
> > +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> > @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> > /** Create file at utf8-encoded @a file with contents @a contents.
> > * @a file must not already exist.
> > * Use @a pool for memory allocations.
> > + *
> > + * @since New in 1.7.
> > + */
> > +svn_error_t *
> > +svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *scratch_pool);
> > +
> > +/** Like svn_io_file_create2(), but with a C string instead
> > + * of an #svn_string_t.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.6 API.
> > */
> > +SVN_DEPRECATED
> > svn_error_t *
> > svn_io_file_create(const char *file,
> > const char *contents,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
> >
> > /*** From io.c ***/
> > svn_error_t *
> > +svn_io_file_create(const char *file,
> > + const char *contents,
> > + apr_pool_t *pool)
> > +{
> > + const svn_string_t *contents_string;
> > +
> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> > + return svn_io_file_create2(file, contents_string, pool);
> > +}
> > +
> > +svn_error_t *
> > svn_io_open_unique_file2(apr_file_t **file,
> > const char **temp_path,
> > const char *path,
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/io.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> > @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> > return SVN_NO_ERROR;
> > }
> >
> > -svn_error_t *svn_io_file_create(const char *file,
> > - const char *contents,
> > - apr_pool_t *pool)
> > +svn_error_t *svn_io_file_create2(const char *file,
> > + const svn_string_t *contents,
> > + apr_pool_t *pool)
> > {
> > apr_file_t *f;
> > apr_size_t written;
> > @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> > (APR_WRITE | APR_CREATE | APR_EXCL),
> > APR_OS_DEFAULT,
> > pool));
> > - if (contents && *contents)
> > - err = svn_io_file_write_full(f, contents, strlen(contents),
> > + if (contents && contents->len)
> > + err = svn_io_file_write_full(f, contents->data, contents->len,
> > &written, pool);
> >
> >
> >
> >
>
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Julian Foad <ju...@wandisco.com>.
Not directly related to your change, but I notice the impl. allows the
passed-in string to be null if not wanted. Maybe the doc string should
promise that, otherwise why bother?
- Julian
On Tue, 2011-05-17 at 10:55 +0000, danielsh@apache.org wrote:
> Author: danielsh
> Date: Tue May 17 10:55:51 2011
> New Revision: 1104124
>
> URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> Log:
> Revv the svn_io_file_create() API to take non-NUL-terminated strings.
>
> * subversion/include/svn_io.h
> (svn_io_file_create2): New.
> (svn_io_file_create): Deprecate.
>
> * subversion/libsvn_subr/io.c
> (svn_io_file_create2): Renamed from svn_io_file_create().
>
> * subversion/libsvn_subr/deprecated.c
> (svn_io_file_create): New wrapper.
>
> Modified:
> subversion/trunk/subversion/include/svn_io.h
> subversion/trunk/subversion/libsvn_subr/deprecated.c
> 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=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_io.h (original)
> +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> /** Create file at utf8-encoded @a file with contents @a contents.
> * @a file must not already exist.
> * Use @a pool for memory allocations.
> + *
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *scratch_pool);
> +
> +/** Like svn_io_file_create2(), but with a C string instead
> + * of an #svn_string_t.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_io_file_create(const char *file,
> const char *contents,
>
> Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>
> /*** From io.c ***/
> svn_error_t *
> +svn_io_file_create(const char *file,
> + const char *contents,
> + apr_pool_t *pool)
> +{
> + const svn_string_t *contents_string;
> +
> + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> + return svn_io_file_create2(file, contents_string, pool);
> +}
> +
> +svn_error_t *
> svn_io_open_unique_file2(apr_file_t **file,
> const char **temp_path,
> const char *path,
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> return SVN_NO_ERROR;
> }
>
> -svn_error_t *svn_io_file_create(const char *file,
> - const char *contents,
> - apr_pool_t *pool)
> +svn_error_t *svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *pool)
> {
> apr_file_t *f;
> apr_size_t written;
> @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> (APR_WRITE | APR_CREATE | APR_EXCL),
> APR_OS_DEFAULT,
> pool));
> - if (contents && *contents)
> - err = svn_io_file_write_full(f, contents, strlen(contents),
> + if (contents && contents->len)
> + err = svn_io_file_write_full(f, contents->data, contents->len,
> &written, pool);
>
>
>
>
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Tue, May 17, 2011 at 07:12:58 -0400:
> On Tue, May 17, 2011 at 06:55, <da...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> > @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
> >
> > /*** From io.c ***/
> > svn_error_t *
> > +svn_io_file_create(const char *file,
> > + const char *contents,
> > + apr_pool_t *pool)
> > +{
> > + const svn_string_t *contents_string;
> > +
> > + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> > + return svn_io_file_create2(file, contents_string, pool);
>
> contents_string can go on the stack, rather than allocated:
>
> contents_string.data = contents;
> contents_string.len = strlen(contents);
> return svn_error_return(svn_io_file_create2(file, &contents_string, pool);
>
> ... tho it makes the NULL concept a bit more difficult.
>
I didn't realize that svn_string_*create() didn't do that for me.
The patch seems straightforward,
[[[
Index: subversion/libsvn_subr/deprecated.c
===================================================================
--- subversion/libsvn_subr/deprecated.c (revision 1104124)
+++ subversion/libsvn_subr/deprecated.c (working copy)
@@ -634,10 +634,17 @@ svn_io_file_create(const char *file,
const char *contents,
apr_pool_t *pool)
{
- const svn_string_t *contents_string;
+ if (contents && *contents)
+ {
+ const svn_string_t contents_string;
+ contents_string.data = contents;
+ contents_string.len = strlen(contents);
+ return svn_io_file_create2(file, &contents_string, pool);
+ }
+ else
+ {
+ return svn_io_file_create2(file, NULL, pool);
+ }
-
- contents_string = (contents ? svn_string_create(contents, pool) : NULL);
- return svn_io_file_create2(file, contents_string, pool);
}
svn_error_t *
]]]
but I wonder if we shouldn't be teaching the svn_string_create() API to
optionally return a shallow copy. (which isn't as good as stack, but
still saves the pstrdup())
> >...
>
> Cheers,
> -g
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, May 17, 2011 at 06:55, <da...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>
> /*** From io.c ***/
> svn_error_t *
> +svn_io_file_create(const char *file,
> + const char *contents,
> + apr_pool_t *pool)
> +{
> + const svn_string_t *contents_string;
> +
> + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> + return svn_io_file_create2(file, contents_string, pool);
contents_string can go on the stack, rather than allocated:
contents_string.data = contents;
contents_string.len = strlen(contents);
return svn_error_return(svn_io_file_create2(file, &contents_string, pool);
... tho it makes the NULL concept a bit more difficult.
>...
Cheers,
-g
Re: svn commit: r1104124 - in /subversion/trunk/subversion:
include/svn_io.h libsvn_subr/deprecated.c libsvn_subr/io.c
Posted by Julian Foad <ju...@wandisco.com>.
Not directly related to your change, but I notice the impl. allows the
passed-in string to be null if not wanted. Maybe the doc string should
promise that, otherwise why bother?
- Julian
On Tue, 2011-05-17 at 10:55 +0000, danielsh@apache.org wrote:
> Author: danielsh
> Date: Tue May 17 10:55:51 2011
> New Revision: 1104124
>
> URL: http://svn.apache.org/viewvc?rev=1104124&view=rev
> Log:
> Revv the svn_io_file_create() API to take non-NUL-terminated strings.
>
> * subversion/include/svn_io.h
> (svn_io_file_create2): New.
> (svn_io_file_create): Deprecate.
>
> * subversion/libsvn_subr/io.c
> (svn_io_file_create2): Renamed from svn_io_file_create().
>
> * subversion/libsvn_subr/deprecated.c
> (svn_io_file_create): New wrapper.
>
> Modified:
> subversion/trunk/subversion/include/svn_io.h
> subversion/trunk/subversion/libsvn_subr/deprecated.c
> 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=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_io.h (original)
> +++ subversion/trunk/subversion/include/svn_io.h Tue May 17 10:55:51 2011
> @@ -645,7 +645,20 @@ svn_io_files_contents_same_p(svn_boolean
> /** Create file at utf8-encoded @a file with contents @a contents.
> * @a file must not already exist.
> * Use @a pool for memory allocations.
> + *
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *scratch_pool);
> +
> +/** Like svn_io_file_create2(), but with a C string instead
> + * of an #svn_string_t.
> + *
> + * @deprecated Provided for backward compatibility with the 1.6 API.
> */
> +SVN_DEPRECATED
> svn_error_t *
> svn_io_file_create(const char *file,
> const char *contents,
>
> Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/deprecated.c Tue May 17 10:55:51 2011
> @@ -630,6 +630,17 @@ svn_opt_print_generic_help(const char *h
>
> /*** From io.c ***/
> svn_error_t *
> +svn_io_file_create(const char *file,
> + const char *contents,
> + apr_pool_t *pool)
> +{
> + const svn_string_t *contents_string;
> +
> + contents_string = (contents ? svn_string_create(contents, pool) : NULL);
> + return svn_io_file_create2(file, contents_string, pool);
> +}
> +
> +svn_error_t *
> svn_io_open_unique_file2(apr_file_t **file,
> const char **temp_path,
> const char *path,
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1104124&r1=1104123&r2=1104124&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Tue May 17 10:55:51 2011
> @@ -1092,9 +1092,9 @@ svn_io_make_dir_recursively(const char *
> return SVN_NO_ERROR;
> }
>
> -svn_error_t *svn_io_file_create(const char *file,
> - const char *contents,
> - apr_pool_t *pool)
> +svn_error_t *svn_io_file_create2(const char *file,
> + const svn_string_t *contents,
> + apr_pool_t *pool)
> {
> apr_file_t *f;
> apr_size_t written;
> @@ -1104,8 +1104,8 @@ svn_error_t *svn_io_file_create(const ch
> (APR_WRITE | APR_CREATE | APR_EXCL),
> APR_OS_DEFAULT,
> pool));
> - if (contents && *contents)
> - err = svn_io_file_write_full(f, contents, strlen(contents),
> + if (contents && contents->len)
> + err = svn_io_file_write_full(f, contents->data, contents->len,
> &written, pool);
>
>
>
>