You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2006/09/26 22:35:43 UTC
svn commit: r450188 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Author: minfrin
Date: Tue Sep 26 13:35:42 2006
New Revision: 450188
URL: http://svn.apache.org/viewvc?view=rev&rev=450188
Log:
Allocate the temporary copy file buffer from a pool.
Modified:
httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?view=diff&rev=450188&r1=450187&r2=450188
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Tue Sep 26 13:35:42 2006
@@ -985,7 +985,8 @@
}
-static apr_status_t copy_body(apr_file_t *srcfd, apr_off_t srcoff,
+static apr_status_t copy_body(apr_pool_t *p,
+ apr_file_t *srcfd, apr_off_t srcoff,
apr_file_t *destfd, apr_off_t destoff,
apr_off_t len)
{
@@ -993,7 +994,11 @@
apr_size_t size;
apr_finfo_t finfo;
apr_time_t starttime = apr_time_now();
- char buf[CACHE_BUF_SIZE];
+
+ char *buf = apr_palloc(p, CACHE_BUF_SIZE);
+ if (!buf) {
+ return APR_ENOMEM;
+ }
if(srcoff != 0) {
rc = apr_file_seek(srcfd, APR_SET, &srcoff);
@@ -1178,7 +1183,7 @@
e = APR_BRIGADE_FIRST(bb);
a = e->data;
- rv = copy_body(a->fd, e->start, dobj->tfd, 0,
+ rv = copy_body(r->pool, a->fd, e->start, dobj->tfd, 0,
dobj->file_size);
if(rv != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,
Re: svn commit: r450188 -
/httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Graham Leggett <mi...@sharp.fm>.
On Wed, September 27, 2006 10:12 am, Ruediger Pluem wrote:
>> We would copy the body once per request, surely? That's how I read it -
>> copy_body would be called once, resulting in the buffer being declared
>> once, and reused inside the copy_body loop.
>
> Yes, of course. Stupid thought of mine. It had been already too late and I
> was too
> tired :-). Thanks for review anyway.
Same problem here... :)
Regards,
Graham
--
Re: svn commit: r450188 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/27/2006 12:22 AM, Graham Leggett wrote:
> Ruediger Pluem wrote:
>
>> Are we sure that we do not iterate too often (> 100) over this during
>> the lifetime
>> of a request? I would say 'No, we do not iterate too often', but I
>> think a crosscheck
>> by someone else is a good idea. Otherwise we would have a potential
>> temporary memory
>> leak here.
>
>
> We would copy the body once per request, surely? That's how I read it -
> copy_body would be called once, resulting in the buffer being declared
> once, and reused inside the copy_body loop.
Yes, of course. Stupid thought of mine. It had been already too late and I was too
tired :-). Thanks for review anyway.
Regards
Rüdiger
Re: svn commit: r450188 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Niklas Edmundsson <ni...@acc.umu.se>.
On Wed, 27 Sep 2006, Graham Leggett wrote:
> Ruediger Pluem wrote:
>
>> Are we sure that we do not iterate too often (> 100) over this during the
>> lifetime
>> of a request? I would say 'No, we do not iterate too often', but I think a
>> crosscheck
>> by someone else is a good idea. Otherwise we would have a potential
>> temporary memory
>> leak here.
>
> We would copy the body once per request, surely? That's how I read it -
> copy_body would be called once, resulting in the buffer being declared once,
> and reused inside the copy_body loop.
The code is very picky about there only being a single, complete,
body so it should only be called once per request.
/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se | nikke@acc.umu.se
---------------------------------------------------------------------------
I am Jay Leno of Borg : Kevin's love life is irrelevant (and non-existing)
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Re: svn commit: r450188 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Graham Leggett <mi...@sharp.fm>.
Ruediger Pluem wrote:
> Are we sure that we do not iterate too often (> 100) over this during the lifetime
> of a request? I would say 'No, we do not iterate too often', but I think a crosscheck
> by someone else is a good idea. Otherwise we would have a potential temporary memory
> leak here.
We would copy the body once per request, surely? That's how I read it -
copy_body would be called once, resulting in the buffer being declared
once, and reused inside the copy_body loop.
Regards,
Graham
--
Re: svn commit: r450188 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09/26/2006 10:35 PM, wrote:
> Author: minfrin
> Date: Tue Sep 26 13:35:42 2006
> New Revision: 450188
>
>
> Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?view=diff&rev=450188&r1=450187&r2=450188
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
> +++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Tue Sep 26 13:35:42 2006
> @@ -985,7 +985,8 @@
> }
>
>
> -static apr_status_t copy_body(apr_file_t *srcfd, apr_off_t srcoff,
> +static apr_status_t copy_body(apr_pool_t *p,
> + apr_file_t *srcfd, apr_off_t srcoff,
> apr_file_t *destfd, apr_off_t destoff,
> apr_off_t len)
> {
> @@ -993,7 +994,11 @@
> apr_size_t size;
> apr_finfo_t finfo;
> apr_time_t starttime = apr_time_now();
> - char buf[CACHE_BUF_SIZE];
> +
> + char *buf = apr_palloc(p, CACHE_BUF_SIZE);
Are we sure that we do not iterate too often (> 100) over this during the lifetime
of a request? I would say 'No, we do not iterate too often', but I think a crosscheck
by someone else is a good idea. Otherwise we would have a potential temporary memory
leak here.
Regards
Rüdiger
Re: svn commit: r450188 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Posted by Davi Arnaut <da...@haxent.com.br>.
On 26/09/2006, at 17:35, minfrin@apache.org wrote:
> Author: minfrin
> Date: Tue Sep 26 13:35:42 2006
> New Revision: 450188
>
> +
> + char *buf = apr_palloc(p, CACHE_BUF_SIZE);
> + if (!buf) {
> + return APR_ENOMEM;
> + }
IIRC, apache abort()s on memory allocation errors.
--
Davi Arnaut