You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2005/08/08 14:11:39 UTC
Re: svn commit: r230733 -
/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
At 09:51 PM 8/7/2005, wrowe@apache.org wrote:
>Author: wrowe
>Date: Sun Aug 7 19:51:32 2005
>New Revision: 230733
>
>URL: http://svn.apache.org/viewcvs?rev=230733&view=rev
>Log:
>
> Fix a double-termination case in svn trunk/; we terminated the
> headers up-front knowing the resulting headers were already
> correctly composed.
>
>--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
>+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Sun Aug 7 19:51:32 2005
>@@ -398,7 +398,6 @@
> /* we never sent the header brigade since there was no request
> * body; send it now with the flush flag
> */
>- terminate_headers(bucket_alloc, header_brigade);
Joe - this was an evil thing; try your regressions now. I expect
you will be happier. But I'm not even sure if you ever touched
this specific case.
My other thought is the choice of bucket allocators; notice that
we seem to be using both r->connection->bucket_alloc, and also
origin->bucket->alloc. I don't believe this is a new flaw.
Thanks to both you and Andre for catching a few sprintf style
formatting bugs (misplaced commas) while I was on vacation;
they are correct in the branch created for evaluating the
backport. Not certain if that too was part of your failures.
If you can provide an update on any segfaults you see on svn
trunk/, as well as the t/proxy suite results, that would be
fantastic.
Bill
Re: svn commit: r230733 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Aug 09, 2005 at 11:40:36AM -0500, William Rowe wrote:
> At 04:38 AM 8/9/2005, Joe Orton wrote:
>
> >Great, yes with last night's regression run the segfaults with worker
> >were gone and the tests are all passing for prefork and worker again on
> >the trunk - thanks a lot Bill. -Werror builds are failing though:
> >
> >mod_proxy_http.c: In function `ap_proxy_http_request':
> >mod_proxy_http.c:312: warning: 'status' might be used uninitialized in
> >this function
>
> Does this patch resolve status for your compiler's -Werror?
> (already committed to trunk, will hold off on proxyreq-2.0.x
> till I hear back.)
No, that didn't fix it. gcc knows that the ap_assert would call a
function which never returns, so that code path would not fall through
any further. I committed a fix - the line number is key in gcc warnings
not the function name, since the latter gets screwed up by all the
inlining which goes on.
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 9 07:54:46 2005
> @@ -949,7 +949,8 @@
> || (bytes_read > 0));
> break;
> default:
> - ap_assert(1 != 1);
> + /* shouldn't be possible */
> + status = APR_EINVAL;
> break;
> }
>
>
Re: svn commit: r230733 -
/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 04:38 AM 8/9/2005, Joe Orton wrote:
>Great, yes with last night's regression run the segfaults with worker
>were gone and the tests are all passing for prefork and worker again on
>the trunk - thanks a lot Bill. -Werror builds are failing though:
>
>mod_proxy_http.c: In function `ap_proxy_http_request':
>mod_proxy_http.c:312: warning: 'status' might be used uninitialized in
>this function
Does this patch resolve status for your compiler's -Werror?
(already committed to trunk, will hold off on proxyreq-2.0.x
till I hear back.)
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 9 07:54:46 2005
@@ -949,7 +949,8 @@
|| (bytes_read > 0));
break;
default:
- ap_assert(1 != 1);
+ /* shouldn't be possible */
+ status = APR_EINVAL;
break;
}
Re: svn commit: r230733 -
/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 04:38 AM 8/9/2005, Joe Orton wrote:
>Great, yes with last night's regression run the segfaults with worker
>were gone and the tests are all passing for prefork and worker again on
>the trunk - thanks a lot Bill. -Werror builds are failing though:
>
>mod_proxy_http.c: In function `ap_proxy_http_request':
>mod_proxy_http.c:312: warning: 'status' might be used uninitialized in
>this function
Does this patch resolve status for your compiler's -Werror?
(already committed to trunk, will hold off on proxyreq-2.0.x
till I hear back.)
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue Aug 9 07:54:46 2005
@@ -949,7 +949,8 @@
|| (bytes_read > 0));
break;
default:
- ap_assert(1 != 1);
+ /* shouldn't be possible */
+ status = APR_EINVAL;
break;
}
Re: svn commit: r230733 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Aug 08, 2005 at 07:11:39AM -0500, William Rowe wrote:
> At 09:51 PM 8/7/2005, wrowe@apache.org wrote:
> >Author: wrowe
> >Date: Sun Aug 7 19:51:32 2005
> >New Revision: 230733
> >
> >URL: http://svn.apache.org/viewcvs?rev=230733&view=rev
> >Log:
> >
> > Fix a double-termination case in svn trunk/; we terminated the
> > headers up-front knowing the resulting headers were already
> > correctly composed.
> >
> >--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> >+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Sun Aug 7 19:51:32 2005
> >@@ -398,7 +398,6 @@
> > /* we never sent the header brigade since there was no request
> > * body; send it now with the flush flag
> > */
> >- terminate_headers(bucket_alloc, header_brigade);
>
> Joe - this was an evil thing; try your regressions now. I expect
> you will be happier. But I'm not even sure if you ever touched
> this specific case.
Great, yes with last night's regression run the segfaults with worker
were gone and the tests are all passing for prefork and worker again on
the trunk - thanks a lot Bill. -Werror builds are failing though:
mod_proxy_http.c: In function `ap_proxy_http_request':
mod_proxy_http.c:312: warning: 'status' might be used uninitialized in
this function
joe
Re: svn commit: r230733 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Aug 08, 2005 at 07:11:39AM -0500, William Rowe wrote:
> At 09:51 PM 8/7/2005, wrowe@apache.org wrote:
> >Author: wrowe
> >Date: Sun Aug 7 19:51:32 2005
> >New Revision: 230733
> >
> >URL: http://svn.apache.org/viewcvs?rev=230733&view=rev
> >Log:
> >
> > Fix a double-termination case in svn trunk/; we terminated the
> > headers up-front knowing the resulting headers were already
> > correctly composed.
> >
> >--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> >+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Sun Aug 7 19:51:32 2005
> >@@ -398,7 +398,6 @@
> > /* we never sent the header brigade since there was no request
> > * body; send it now with the flush flag
> > */
> >- terminate_headers(bucket_alloc, header_brigade);
>
> Joe - this was an evil thing; try your regressions now. I expect
> you will be happier. But I'm not even sure if you ever touched
> this specific case.
Great, yes with last night's regression run the segfaults with worker
were gone and the tests are all passing for prefork and worker again on
the trunk - thanks a lot Bill. -Werror builds are failing though:
mod_proxy_http.c: In function `ap_proxy_http_request':
mod_proxy_http.c:312: warning: 'status' might be used uninitialized in
this function
joe