--------------------- PatchSet 7068 Date: 2005/10/05 12:14:48 Author: adri Branch: tidyup_deferred_reads Tag: (none) Log: Attempt to refactor this code a little. All the code paths inside the modified top level if() construct skip the rest of the function if true, so we can short-circuit things by return;ing early and removing the last else construct. I believe this will make the code easier to read and doesn't change the behaviour of the code. Note: stupid indenting of vi/kate caused this monstrosity. I'll have to run it through indent later. Members: src/http.c:1.17.6.27.4.4->1.17.6.27.4.5 Index: squid/src/http.c =================================================================== RCS file: /cvsroot/squid-sf//squid/src/http.c,v retrieving revision 1.17.6.27.4.4 retrieving revision 1.17.6.27.4.5 diff -u -r1.17.6.27.4.4 -r1.17.6.27.4.5 --- squid/src/http.c 5 Oct 2005 07:44:30 -0000 1.17.6.27.4.4 +++ squid/src/http.c 5 Oct 2005 12:14:48 -0000 1.17.6.27.4.5 @@ -1,6 +1,6 @@ /* - * $Id: http.c,v 1.17.6.27.4.4 2005/10/05 07:44:30 adri Exp $ + * $Id: http.c,v 1.17.6.27.4.5 2005/10/05 12:14:48 adri Exp $ * * DEBUG: section 11 Hypertext Transfer Protocol (HTTP) * AUTHOR: Harvest Derived @@ -645,10 +645,12 @@ fwdFail(httpState->fwd, err); comm_close(fd); } + return; } else if (len == 0 && entry->mem_obj->inmem_hi == 0) { fwdFail(httpState->fwd, errorCon(ERR_ZERO_SIZE_OBJECT, HTTP_BAD_GATEWAY)); httpState->eof = 1; comm_close(fd); + return; } else if (len == 0) { /* Connection closed; retrieval done. */ httpState->eof = 1; @@ -673,131 +675,134 @@ } comm_close(fd); return; - } else { + } + + /* INDENT ONCE START */ if (httpState->reply_hdr_state < 2) { - httpProcessReplyHeader(httpState, buf, len); - if (httpState->reply_hdr_state == 2) { + httpProcessReplyHeader(httpState, buf, len); + if (httpState->reply_hdr_state == 2) { http_status s = entry->mem_obj->reply->sline.status; if (s == HTTP_HEADER_TOO_LARGE) { - debug(11, 1) ("WARNING: %s:%d: HTTP header too large\n", __FILE__, __LINE__); - storeEntryReset(entry); - fwdFail(httpState->fwd, errorCon(ERR_TOO_BIG, HTTP_BAD_GATEWAY)); - httpState->fwd->flags.dont_retry = 1; - comm_close(fd); - return; + debug(11, 1) ("WARNING: %s:%d: HTTP header too large\n", __FILE__, __LINE__); + storeEntryReset(entry); + fwdFail(httpState->fwd, errorCon(ERR_TOO_BIG, HTTP_BAD_GATEWAY)); + httpState->fwd->flags.dont_retry = 1; + comm_close(fd); + return; } if (s == HTTP_INVALID_HEADER && !(entry->mem_obj->reply->sline.version.major == 0 && entry->mem_obj->reply->sline.version.minor == 9)) { - storeEntryReset(entry); - fwdFail(httpState->fwd, errorCon(ERR_INVALID_RESP, HTTP_BAD_GATEWAY)); - httpState->fwd->flags.dont_retry = 1; - comm_close(fd); - return; + storeEntryReset(entry); + fwdFail(httpState->fwd, errorCon(ERR_INVALID_RESP, HTTP_BAD_GATEWAY)); + httpState->fwd->flags.dont_retry = 1; + comm_close(fd); + return; } -#if WIP_FWD_LOG + #if WIP_FWD_LOG fwdStatus(httpState->fwd, s); -#endif + #endif /* - * If its not a reply that we will re-forward, then - * allow the client to get it. - */ + * If its not a reply that we will re-forward, then + * allow the client to get it. + */ if (!fwdReforwardableStatus(s)) - EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT); - } + EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT); + } } storeAppend(entry, buf, len); if (EBIT_TEST(entry->flags, ENTRY_ABORTED)) { - /* - * the above storeAppend() call could ABORT this entry, - * in that case, the server FD should already be closed. - * there's nothing for us to do. - */ - return; + /* + * the above storeAppend() call could ABORT this entry, + * in that case, the server FD should already be closed. + * there's nothing for us to do. + */ + return; } switch (httpPconnTransferDone(httpState)) { case 1: - { + { int keep_alive = 1; /* - * If we didn't send a keep-alive request header, then this - * can not be a persistent connection. - */ + * If we didn't send a keep-alive request header, then this + * can not be a persistent connection. + */ if (!httpState->flags.keepalive) - keep_alive = 0; + keep_alive = 0; /* - * If we haven't sent the whole request then this can not be a persistent - * connection. - */ + * If we haven't sent the whole request then this can not be a persistent + * connection. + */ if (!httpState->flags.request_sent) { - debug(11, 1) ("httpReadReply: Request not yet fully sent \"%s %s\"\n", + debug(11, 1) ("httpReadReply: Request not yet fully sent \"%s %s\"\n", RequestMethodStr[httpState->orig_request->method], storeUrl(entry)); - keep_alive = 0; + keep_alive = 0; } /* - * What does the reply have to say about keep-alive? - */ + * What does the reply have to say about keep-alive? + */ if (!entry->mem_obj->reply->keep_alive) - keep_alive = 0; + keep_alive = 0; /* - * Verify that the connection is clean - */ + * Verify that the connection is clean + */ if (len == read_sz) { - statCounter.syscalls.sock.reads++; - len = FD_READ_METHOD(fd, buf, SQUID_TCP_SO_RCVBUF); - if ((len < 0 && !ignoreErrno(errno)) || len == 0) { + statCounter.syscalls.sock.reads++; + len = FD_READ_METHOD(fd, buf, SQUID_TCP_SO_RCVBUF); + if ((len < 0 && !ignoreErrno(errno)) || len == 0) { keep_alive = 0; - } else if (len > 0) { + } else if (len > 0) { debug(11, Config.onoff.relaxed_header_parser <= 0 || keep_alive ? 1 : 2) - ("httpReadReply: Excess data from \"%s %s\"\n", - RequestMethodStr[httpState->orig_request->method], - storeUrl(entry)); + ("httpReadReply: Excess data from \"%s %s\"\n", + RequestMethodStr[httpState->orig_request->method], + storeUrl(entry)); storeAppend(entry, buf, len); keep_alive = 0; - } + } } if (keep_alive) { - /* yes we have to clear all these! */ - commSetDefer(fd, NULL, NULL); - commSetTimeout(fd, -1, NULL, NULL); - commSetSelect(fd, COMM_SELECT_READ, NULL, NULL, 0); -#if DELAY_POOLS - delayClearNoDelay(fd); -#endif - comm_remove_close_handler(fd, httpStateFree, httpState); - fwdUnregister(fd, httpState->fwd); - pconnPush(fd, request->host, request->port); - fwdComplete(httpState->fwd); - httpState->fd = -1; - httpStateFree(fd, httpState); + /* yes we have to clear all these! */ + commSetDefer(fd, NULL, NULL); + commSetTimeout(fd, -1, NULL, NULL); + commSetSelect(fd, COMM_SELECT_READ, NULL, NULL, 0); + #if DELAY_POOLS + delayClearNoDelay(fd); + #endif + comm_remove_close_handler(fd, httpStateFree, httpState); + fwdUnregister(fd, httpState->fwd); + pconnPush(fd, request->host, request->port); + fwdComplete(httpState->fwd); + httpState->fd = -1; + httpStateFree(fd, httpState); } else { - fwdComplete(httpState->fwd); - comm_close(fd); + fwdComplete(httpState->fwd); + comm_close(fd); } - } - return; + } + return; case 0: - /* Wait for more data or EOF condition */ - if (httpState->flags.keepalive_broken) { + /* Wait for more data or EOF condition */ + if (httpState->flags.keepalive_broken) { commSetTimeout(fd, 10, NULL, NULL); - } else { + } else { commSetTimeout(fd, Config.Timeout.read, NULL, NULL); - } - commSetSelect(fd, COMM_SELECT_READ, httpReadReply, httpState, 0); - return; + } + commSetSelect(fd, COMM_SELECT_READ, httpReadReply, httpState, 0); + return; case -1: - /* Server is nasty on us. Shut down */ - debug(11, Config.onoff.relaxed_header_parser <= 0 || entry->mem_obj->reply->keep_alive ? 1 : 2) + /* Server is nasty on us. Shut down */ + debug(11, Config.onoff.relaxed_header_parser <= 0 || entry->mem_obj->reply->keep_alive ? 1 : 2) ("httpReadReply: Excess data from \"%s %s\"\n", RequestMethodStr[httpState->orig_request->method], storeUrl(entry)); - fwdComplete(httpState->fwd); - comm_close(fd); - return; + fwdComplete(httpState->fwd); + comm_close(fd); + return; default: - fatal("Unexpected httpPconnTransferDone() status\n"); - break; + fatal("Unexpected httpPconnTransferDone() status\n"); + break; } - } + /* INDENT ONCE START */ + } /* This will be called when request write is complete. Schedule read of