You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2006/01/02 17:53:01 UTC
svn commit: r365376 -
/httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
Author: jim
Date: Mon Jan 2 08:52:58 2006
New Revision: 365376
URL: http://svn.apache.org/viewcvs?rev=365376&view=rev
Log:
Avoid magic numbers. Since we are reading the header, let's
be explicit about it. Also removes the need to clean up
the readbuf again, and any potential for confusion on
what we are doing ;)
Modified:
httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
Modified: httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c?rev=365376&r1=365375&r2=365376&view=diff
==============================================================================
--- httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c (original)
+++ httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c Mon Jan 2 08:52:58 2006
@@ -454,25 +454,27 @@
int rid, type = 0;
char plen = 0;
apr_bucket *b;
+ fcgi_header rheader;
+ int rheader_size = sizeof(fcgi_header);
memset(readbuf, 0, sizeof(readbuf));
/* First, we grab the header... */
- readbuflen = 8;
+ readbuflen = rheader_size;
- rv = apr_socket_recv(conn->sock, readbuf, &readbuflen);
+ rv = apr_socket_recv(conn->sock, (char *)&rheader, &readbuflen);
if (rv != APR_SUCCESS) {
break;
}
- if (readbuflen != 8) {
+ if (readbuflen != rheader_size) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
"proxy: FCGI: Failed to read entire header");
rv = APR_EINVAL;
break;
}
- if (readbuf[0] != FCGI_VERSION) {
+ if (rheader.version != FCGI_VERSION) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
"proxy: FCGI: Got bogus version %d",
(int) readbuf[0]);
@@ -480,10 +482,10 @@
break;
}
- type = readbuf[1];
+ type = rheader.type;
- rid |= readbuf[2] << 8;
- rid |= readbuf[3] << 0;
+ rid |= rheader.requestIdB1 << 8;
+ rid |= rheader.requestIdB0 << 0;
if (rid != request_id) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
@@ -493,13 +495,10 @@
break;
}
- clen |= readbuf[4] << 8;
- clen |= readbuf[5] << 0;
+ clen |= rheader.contentLengthB1 << 8;
+ clen |= rheader.contentLengthB0 << 0;
- plen = readbuf[6];
-
- /* Clear out the header so our buffer is zeroed out again */
- memset(readbuf, 0, 8);
+ plen = rheader.paddingLength;
recv_again:
if (clen > sizeof(readbuf) - 1) {
Re: svn commit: r365376 - /httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 1/2/06, jim@apache.org <ji...@apache.org> wrote:
> Author: jim
> Date: Mon Jan 2 08:52:58 2006
> New Revision: 365376
>
> URL: http://svn.apache.org/viewcvs?rev=365376&view=rev
> Log:
> Avoid magic numbers. Since we are reading the header, let's
> be explicit about it. Also removes the need to clean up
> the readbuf again, and any potential for confusion on
> what we are doing ;)
Um, doesn't this assume that there's no padding in the compiler's
layout of the struct? Is that safe to do? I've always been under the
impression that relying on such things is not portable. If nothing
else we should probably have a compile time check that it's size == 8,
or something...
-garrett
Re: svn commit: r365376 - /httpd/httpd/branches/fcgi-proxy-dev/modules/proxy/mod_proxy_fcgi.c
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 1/2/06, jim@apache.org <ji...@apache.org> wrote:
> Author: jim
> Date: Mon Jan 2 08:52:58 2006
> New Revision: 365376
>
> URL: http://svn.apache.org/viewcvs?rev=365376&view=rev
> Log:
> Avoid magic numbers. Since we are reading the header, let's
> be explicit about it. Also removes the need to clean up
> the readbuf again, and any potential for confusion on
> what we are doing ;)
Um, doesn't this assume that there's no padding in the compiler's
layout of the struct? Is that safe to do? I've always been under the
impression that relying on such things is not portable. If nothing
else we should probably have a compile time check that it's size == 8,
or something...
-garrett