Ruediger Pluem wrote:
On 06/27/2007 05:51 PM, Jim Jagielski wrote:
On Jun 27, 2007, at 11:08 AM, Nick Kew wrote:
On Wed, 27 Jun 2007 14:17:36 -0000
[EMAIL PROTECTED] wrote:
+ * mod_proxy: Arrange the timeout handling.
+ Trunk version of patch:
+ http://svn.apache.org/viewvc?view=rev&revision=550514
+ http://svn.apache.org/viewvc?view=rev&revision=546128
+ +1: jfclere
Looks reasonable, but is there a reference to the problem it solves?
+
+ * mod_proxy: Improve traces in ap_proxy_http_process_response()
+ to investigate PR37770.
+ Trunk version of patch:
+ http://svn.apache.org/viewvc?view=rev&rev=549420
+ +1: jfclere
Hmmm. This is designed to improve an error message?
+ tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+ rv = ap_rgetline(&tmp_s, n, &len, r, fold, tmp_bb);
+ apr_brigade_destroy(tmp_bb);
Isn't a whole new brigade an unnecessarily overhead (and
potentially large if the function gets used more in future)?
What problem does it solve?
Yeah... all this just so we can see the return val
of ap_rgetline()??
Yes, but have a look at ap_getline in protocol.c which was used before instead
of ap_proxygetline. It does exactly the same thing regarding the brigade.
So his solution is not less efficient than the old one. Of course it can be
discussed
whether the old one was efficient enough :-).
I guess we can create a brigade for this purpose at the beginning of
ap_proxy_http_process_response pass it to ap_proxygetline and do a
apr_brigade_cleanup each time
after we have called ap_proxygetline. But in this case we can also use
ap_rgetline directly in ap_proxy_http_process_response
I like the idea.
Cheers
Jean-Frederic
as there is not much left of
ap_proxygetline in this case.
The other aspect of this patch is the matter of code duplication. The question
is
whether it is good to nearly duplicate the code of ap_getline and thus have to
manage
it twice in different locations.
Regards
Ru"diger
