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

[PATCH 2] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?

From: Patrick Mayweg <mayweg_at_qint.de>
Date: 2005-05-08 16:04:25 CEST

Hi,
this patch removes the last of the SVN_STREAM_CHUNK_SIZE buffers from
the stack and use the pool instead. This fixes the stack overflow for
the javahl binding on windows. I am not sure if this change is
permissible, because of the interface change.
Regards,
Patrick Mayweg

Log message:
12345678901234567890123456789012345678901234567890123456789012345678901234567890

[[[
Use the pool memory instead of stack memory for big buffers in
svn_subst_translate_stream, which needs a new parameter.

* subversion/include/svn_subst.h
  (svn_subst_translate_stream) : add new apr_pool parameter.

* subversion/libsvn_subr/subst.c
  (svn_subst_translate_stream) : add new apr_pool parameter and use it
allocate
   a big buffer from that pool.
  (svn_subst_copy_and_translate2, svn_subst_copy_and_translate) : add pool
   parameter to svn_subst_translate_stream call.

* subversion/libsvn_wc/props.c
  (validate_eol_prop_against_file) : add pool parameter to
   svn_subst_translate_stream call.

* subversion/libsvn_client/cat.c
  (cat_local_file,svn_client_cat2) : add pool parameter to
   svn_subst_translate_stream call.
]]]

Patrick Mayweg wrote:

> Hi Philip,
>
> Philip Martin wrote:
>
>> "Peter N. Lundblad" <peter@famlundblad.se> writes:
>>
>>
>>
>>> Ouch! Putting vars of those sizes on the stack isn't very friendly.
>>> Just
>>> allocate it out of a pool instead.
>>>
>>> (Saying this without having investigated in detail, but I can't
>>> imagine it
>>> should be a problem.)
>>>
>>
>>
>> Look at this:
>>
>>
> This is the only case, where a pool is not available.
>
>> svn_error_t *
>> svn_subst_translate_stream (svn_stream_t *s, /* src stream */
>> svn_stream_t *d, /* dst stream */
>> const char *eol_str,
>> svn_boolean_t repair,
>> const svn_subst_keywords_t *keywords,
>> svn_boolean_t expand)
>> {
>> char buf[SVN_STREAM_CHUNK_SIZE + 1];
>> const char *p, *interesting;
>>
>> There is no readily available pool, and neither svn_stream_t nor
>> svn_subst_keywords_t provides one. There is a void* baton in
>> svn_stream_t but svn_subst_translate_stream can't really make any
>> assumptions about it.
>>
>>
> All callers of this function have a pool available. If I could extend
> the interface by a pool parameter, I could change that case too.
> Patrick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>

Index: subversion/libsvn_client/cat.c
===================================================================
--- subversion/libsvn_client/cat.c (revision 14544)
+++ subversion/libsvn_client/cat.c (working copy)
@@ -137,7 +137,8 @@
                              APR_READ, APR_OS_DEFAULT, pool));
   input = svn_stream_from_aprfile (input_file, pool);
 
- SVN_ERR (svn_subst_translate_stream (input, output, eol, FALSE, &kw, TRUE));
+ SVN_ERR (svn_subst_translate_stream (input, output, eol, FALSE, &kw, TRUE,
+ pool));
 
   SVN_ERR (svn_stream_close (input));
   SVN_ERR (svn_io_file_close (input_file, pool));
@@ -262,7 +263,7 @@
         }
 
       SVN_ERR (svn_subst_translate_stream (tmp_stream, out, eol, FALSE, &kw,
- TRUE));
+ TRUE, pool));
 
       SVN_ERR (svn_stream_close (tmp_stream));
     }
Index: subversion/libsvn_subr/subst.c
===================================================================
--- subversion/libsvn_subr/subst.c (revision 14544)
+++ subversion/libsvn_subr/subst.c (working copy)
@@ -591,9 +591,10 @@
                             const char *eol_str,
                             svn_boolean_t repair,
                             const svn_subst_keywords_t *keywords,
- svn_boolean_t expand)
+ svn_boolean_t expand,
+ apr_pool_t *pool)
 {
- char buf[SVN_STREAM_CHUNK_SIZE + 1];
+ char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE + 1);
   const char *p, *interesting;
   apr_size_t len, readlen;
   apr_size_t eol_str_len = eol_str ? strlen (eol_str) : 0;
@@ -731,7 +732,7 @@
 
   /* Translate src stream into dst stream. */
   err = svn_subst_translate_stream (src_stream, dst_stream,
- eol_str, repair, keywords, expand);
+ eol_str, repair, keywords, expand, pool);
   if (err)
     {
       svn_error_clear (svn_stream_close (src_stream));
@@ -973,7 +974,7 @@
 
   /* Translate src stream into dst stream. */
   err = svn_subst_translate_stream (src_stream, dst_stream,
- eol_str, repair, keywords, expand);
+ eol_str, repair, keywords, expand, pool);
   if (err)
     {
       if (err->apr_err == SVN_ERR_IO_INCONSISTENT_EOL)
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 14544)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -975,7 +975,7 @@
      endings. The function is "translating" to an empty stream. This
      is sneeeeeeeeeeeaky. */
   err = svn_subst_translate_stream (read_stream, write_stream,
- "", FALSE, NULL, FALSE);
+ "", FALSE, NULL, FALSE, pool);
   if (err && err->apr_err == SVN_ERR_IO_INCONSISTENT_EOL)
     return svn_error_createf (SVN_ERR_ILLEGAL_TARGET, err,
                               _("File '%s' has inconsistent newlines"),
Index: subversion/include/svn_subst.h
===================================================================
--- subversion/include/svn_subst.h (revision 14544)
+++ subversion/include/svn_subst.h (working copy)
@@ -164,7 +164,8 @@
                             const char *eol_str,
                             svn_boolean_t repair,
                             const svn_subst_keywords_t *keywords,
- svn_boolean_t expand);
+ svn_boolean_t expand,
+ apr_pool_t *pool);
 
 
 /**

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 8 16:05:14 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.