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

Re: [PATCH 1] V2 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-09 08:55:25 CEST

Hi Jani,
thanks for pointing that out. I have made a new patch.
Regards,
Patrick

Log message:
[[[
Use the pool memory instead of stack memory for big buffers in all
cases, which
do not need an interface change.

* subversion/libsvn_ra_dav/util.c
 (parse_spool_file) : use pool memory for big buffer.

* subversion/libsvn_repos/reporter.c
 (compare_files) : use pool memory for big buffers.

* subversion/libsvn_repos/delta.c
 (compare_files) : use pool memory for big buffers.

* subversion/libsvn_subr/stream.c
 (svn_stream_copy) : use pool memory for big buffer.
]]]

Jani Averbach wrote:

>On 2005-05-08 16:03+0200, Patrick Mayweg wrote:
>
>
>>Hi,
>>this patch removes all but one 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 have two comments:
>
>
>
>>Index: subversion/libsvn_ra_dav/util.c
>>===================================================================
>>--- subversion/libsvn_ra_dav/util.c (revision 14544)
>>+++ subversion/libsvn_ra_dav/util.c (working copy)
>>@@ -502,7 +502,7 @@
>> {
>> apr_file_t *spool_file;
>> svn_stream_t *spool_stream;
>>- char buf[SVN_STREAM_CHUNK_SIZE];
>>+ char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE);
>> apr_size_t len;
>>
>> SVN_ERR( svn_io_file_open(&spool_file, spool_file_name,
>>
>>
>
>Later in the same function there is code like this:
>
> len = sizeof (buf);
> ...
> if (len != sizeof (buf))
>
>This won't work with dynamically allocated buf.
>
>
>
>
>>Index: subversion/libsvn_subr/stream.c
>>===================================================================
>>--- subversion/libsvn_subr/stream.c (revision 14544)
>>+++ subversion/libsvn_subr/stream.c (working copy)
>>@@ -183,7 +183,7 @@
>> svn_error_t *svn_stream_copy (svn_stream_t *from, svn_stream_t *to,
>> apr_pool_t *pool)
>> {
>>- char buf[SVN_STREAM_CHUNK_SIZE];
>>+ char *buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
>> apr_size_t len;
>>
>>
>>
>
>And same goes here:
>
> while (1)
> {
> len = sizeof (buf);
> ...
>
>BR, Jani
>
>
>

Index: subversion/libsvn_ra_dav/util.c
===================================================================
--- subversion/libsvn_ra_dav/util.c (revision 14639)
+++ subversion/libsvn_ra_dav/util.c (working copy)
@@ -502,7 +502,7 @@
 {
   apr_file_t *spool_file;
   svn_stream_t *spool_stream;
- char buf[SVN_STREAM_CHUNK_SIZE];
+ char *buf = apr_palloc(pool, SVN_STREAM_CHUNK_SIZE);
   apr_size_t len;
   
   SVN_ERR( svn_io_file_open(&spool_file, spool_file_name,
@@ -510,7 +510,7 @@
   spool_stream = svn_stream_from_aprfile(spool_file, pool);
   while (1)
     {
- len = sizeof (buf);
+ len = SVN_STREAM_CHUNK_SIZE;
       SVN_ERR (svn_stream_read (spool_stream, buf, &len));
       if (len > 0)
         ne_xml_parse(success_parser, buf, len);
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 14544)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -448,7 +448,7 @@
   svn_filesize_t size1, size2;
   unsigned char digest1[APR_MD5_DIGESTSIZE], digest2[APR_MD5_DIGESTSIZE];
   svn_stream_t *stream1, *stream2;
- char buf1[SVN_STREAM_CHUNK_SIZE], buf2[SVN_STREAM_CHUNK_SIZE];
+ char *buf1, *buf2;
   apr_size_t len1, len2;
 
   /* If the filesystem claims the things haven't changed, then they
@@ -487,6 +487,8 @@
   SVN_ERR (svn_fs_file_contents (&stream1, root1, path1, pool));
   SVN_ERR (svn_fs_file_contents (&stream2, root2, path2, pool));
 
+ buf1 = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
+ buf2 = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
   do
     {
       len1 = len2 = SVN_STREAM_CHUNK_SIZE;
Index: subversion/libsvn_repos/delta.c
===================================================================
--- subversion/libsvn_repos/delta.c (revision 14544)
+++ subversion/libsvn_repos/delta.c (working copy)
@@ -616,7 +616,7 @@
   svn_filesize_t size1, size2;
   unsigned char digest1[APR_MD5_DIGESTSIZE], digest2[APR_MD5_DIGESTSIZE];
   svn_stream_t *stream1, *stream2;
- char buf1[SVN_STREAM_CHUNK_SIZE], buf2[SVN_STREAM_CHUNK_SIZE];
+ char *buf1, *buf2;
   apr_size_t len1, len2;
 
   /* If the filesystem claims the things haven't changed, then they
@@ -655,6 +655,8 @@
   SVN_ERR (svn_fs_file_contents (&stream1, root1, path1, pool));
   SVN_ERR (svn_fs_file_contents (&stream2, root2, path2, pool));
 
+ buf1 = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
+ buf2 = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
   do
     {
       len1 = len2 = SVN_STREAM_CHUNK_SIZE;
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 14639)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -208,7 +208,7 @@
 svn_error_t *svn_stream_copy (svn_stream_t *from, svn_stream_t *to,
                               apr_pool_t *pool)
 {
- char buf[SVN_STREAM_CHUNK_SIZE];
+ char *buf = apr_palloc (pool, SVN_STREAM_CHUNK_SIZE);
   apr_size_t len;
 
   /* Read and write chunks until we get a short read, indicating the
@@ -216,7 +216,7 @@
      associated error.) */
   while (1)
     {
- len = sizeof (buf);
+ len = SVN_STREAM_CHUNK_SIZE;
       SVN_ERR (svn_stream_read (from, buf, &len));
       if (len > 0)
         SVN_ERR (svn_stream_write (to, buf, &len));

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon May 9 08:56:20 2005

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