You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Patrick Mayweg <ma...@qint.de> on 2005/05/08 14:03:58 UTC
[PATCH 1] Allocating large buffers on the stack was: Re: Subversion
1.2.0 RC3 (final candidate) on Monday?
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.
Regards,
Patrick Mayweg
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.
]]]
Patrick Mayweg wrote:
> Hi Philip,
>
> Philip Martin wrote:
>
>> "Peter N. Lundblad" <pe...@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
>
Re: [PATCH 1] V2 Allocating large buffers on the stack was: Re: Subversion
1.2.0 RC3 (final candidate) on Monday?
Posted by Patrick Mayweg <ma...@qint.de>.
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
>
>
>
Re: [PATCH 1] Allocating large buffers on the stack was: Re: Subversion 1.2.0 RC3 (final candidate) on Monday?
Posted by Jani Averbach <ja...@jaa.iki.fi>.
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
--
Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org