httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Malte S. Stretz" <...@apache.org>
Subject Re: svn commit: r1040177 - /httpd/httpd/trunk/modules/http/http_protocol.c
Date Fri, 03 Dec 2010 09:45:59 GMT
On Friday 03 December 2010 09:52:06 Guenter Knauf wrote:
> Am 02.12.2010 10:39, schrieb Joe Orton:
> > On Mon, Nov 29, 2010 at 04:37:49PM -0000, fuankg@apache.org wrote:
> >> +#ifdef __WATCOMC__
> >> +#pragma disable_message(105)
> >> +#endif
> > 
> > Eww.  Do you really need to litter the source code with this stuff? 
> > Can you not flip compiler switches in the Makefiles?  Regards, Joe
> 
> no. If I use a compiler switch it aplies for all source files which is
> certainly not what I want - instead I only want to suppress this one
> warning about assignment inside condition where the comment states that
> its ugly but wanted, while at all other places the very same warning is
> important and useful since you can just acciedently miss a 2nd '='
> where you dont want to assign but only compare.
> I even expect to add more pragmas for other compilers, f.e. gcc, at
> this place if we would compile with all possible warnings (maybe -Wall
> is already enough).

I recently stumbled upon that big condition and ran away screaming.  
Wouldn't it be better just to refactor it so it is better to read and 
debug?  Some thing like the following (untested, just hacked down) 
shouldn't be more inefficient and even shows that some cases like the 
check for HTTP/1.1 is done twice and could be factored out.  It also shows 
when the chunked flag is actually set (because the last check is never 
evaluated if all the other or'ed stuff is already true, if I got this 
right).

----8<----
int ka = 1;
ka &&= r->connection->keepalive != AP_CONN_CLOSE;
ka &&= !r->expecting_100;
if (!((r->status == HTTP_NOT_MODIFIED)
    || (r->status == HTTP_NO_CONTENT)
    || !r->expecting_100
    || r->header_only
    || apr_table_get(r->headers_out, "Content-Length")
    || ap_find_last_token(r->pool,
          apr_table_get(r->headers_out, "Transfer-Encoding"),
          "chunked"))) {
    if (r->proto_num >= HTTP_VERSION(1,1)) {
        r->chunked = 1;
    }
    else {
        ka = 0;
    }
}
ka &&= r->server->keep_alive;
ka &&= r->server->keep_alive_timeout > 0;
ka &&= (r->server->keep_alive_max == 0) || (left > 0);
ka &&= !ap_status_drops_connection(r->status);
ka &&= !wimpy;
ka &&= !ap_find_token(r->pool, conn, "close");
ka &&= (!apr_table_get(r->subprocess_env, "nokeepalive")
       || apr_table_get(r->headers_in, "Via"));
ka && ((ka_sent = ap_find_token(r->pool, conn, "keep-alive"))
     || (r->proto_num >= HTTP_VERSION(1,1)));
ka &&= is_mpm_running());

if (ka) { //...
----8<----

Cheers,
Malte

Mime
View raw message