You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Brad Nicholes <BN...@novell.com> on 2006/01/26 19:02:14 UTC

Re: svn commit: r372037 - in /httpd/httpd/branches/2.0.x: CHANGES STATUSserver/core.c

>>> On 1/24/2006 at 3:45:47 pm, in message
<20...@minotaur.apache.org>, colm@apache.org
wrote:
> Author: colm
> Date: Tue Jan 24 14:45:43 2006
> New Revision: 372037
> 
> URL: http://svn.apache.org/viewcvs?rev=372037&view=rev 
> Log:
> Backport the NET_TIME elimination fix.
> 
> Submitted by: wrowe
> 
> Modified:
>     httpd/httpd/branches/2.0.x/CHANGES
>     httpd/httpd/branches/2.0.x/STATUS
>     httpd/httpd/branches/2.0.x/server/core.c
> 
> Modified: httpd/httpd/branches/2.0.x/server/core.c
> URL: 
>
http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/server/core.c?rev=37

> 2037&r1=372036&r2=372037&view=diff
>
============================================================================
> ==
> --- httpd/httpd/branches/2.0.x/server/core.c (original)
> +++ httpd/httpd/branches/2.0.x/server/core.c Tue Jan 24 14:45:43
2006
> @@ -3679,11 +3679,9 @@
>      }
>  
>      if (mode != AP_MODE_INIT && mode != AP_MODE_EATCRLF) {
> -        if (ctx->first_line) {
> -            apr_socket_timeout_set(ctx->csd, 
> -                                   keptalive
> -                                      ?
f->c->base_server->keep_alive_timeout
> -                                      :
f->c->base_server->timeout);
> +        if (keptalive && ctx->first_line) {
> +            apr_socket_timeout_set(ctx->csd,
> +                                  
f->c->base_server->keep_alive_timeout);
>              ctx->first_line = 0;
>          }
>          else {
> @@ -4493,10 +4491,9 @@
>  static int core_pre_connection(conn_rec *c, void *csd)
>  {
>      core_net_rec *net = apr_palloc(c->pool, sizeof(*net));
> -
> -#ifdef AP_MPM_DISABLE_NAGLE_ACCEPTED_SOCK
>      apr_status_t rv;
>  
> +#ifdef AP_MPM_DISABLE_NAGLE_ACCEPTED_SOCK
>      /* BillS says perhaps this should be moved to the MPMs. Some
OSes
>       * allow listening socket attributes to be inherited by the
>       * accept sockets which means this call only needs to be made
> @@ -4518,6 +4515,20 @@
>                        "apr_socket_opt_set(APR_TCP_NODELAY)");
>      }
>  #endif
> +
> +    /* The core filter requires the timeout mode to be set, which
> +     * incidentally sets the socket to be nonblocking.  If this
> +     * is not initialized correctly, Linux - for example - will
> +     * be initially blocking, while Solaris will be non blocking
> +     * and any initial read will fail.
> +     */
> +    rv = apr_socket_timeout_set(csd, c->base_server->timeout);
> +    if (rv != APR_SUCCESS) {
> +        /* expected cause is that the client disconnected already
*/
> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
> +                     "apr_socket_timeout_set");
> +    }
> +
>      net->c = c;
>      net->in_ctx = NULL;
>      net->out_ctx = NULL

This patch breaks the NetWare build with an "Illegal implicit
conversion"  trying to pass a 'struct conn_rec *' to ap_log_error()
rather than a 'struct server_rec *'.  Seems a little dangerous to be
passing a pointer to a completely different structure.  Either the calls
to ap_log_error() should be removed or a compatible structure pointer
needs to be passed in.

Brad

Re: svn commit: r372037 - in /httpd/httpd/branches/2.0.x: CHANGES STATUSserver/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Brad Nicholes wrote:
> 
> This patch breaks the NetWare build with an "Illegal implicit
> conversion"  trying to pass a 'struct conn_rec *' to ap_log_error()
> rather than a 'struct server_rec *'.  Seems a little dangerous to be
> passing a pointer to a completely different structure.  Either the calls
> to ap_log_error() should be removed or a compatible structure pointer
> needs to be passed in.

Can we use ap_log_cerror() here?

Re: svn commit: r372037 - in /httpd/httpd/branches/2.0.x: CHANGES STATUSserver/core.c

Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Thu, Jan 26, 2006 at 12:15:16PM -0600, William A. Rowe, Jr. wrote:
> >-        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
> >+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c,
> >                      "apr_socket_timeout_set");
> >     }
> 
> For expediency's sake, +1

+1 here also.

-- 
Colm MacCárthaigh                        Public Key: colm+pgp@stdlib.net

Re: svn commit: r372037 - in /httpd/httpd/branches/2.0.x: CHANGES STATUSserver/core.c

Posted by Paul Querna <ch...@force-elite.com>.
William A. Rowe, Jr. wrote:
> Brad Nicholes wrote:
>> Applying the following patch should fix it.
>>
>> Index: core.c
>> ===================================================================
>> --- core.c    (revision 372564)
>> +++ core.c    (working copy)
>> @@ -4525,7 +4525,7 @@
>>      rv = apr_socket_timeout_set(csd, c->base_server->timeout);
>>      if (rv != APR_SUCCESS) {
>>          /* expected cause is that the client disconnected already */
>> -        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
>> +        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c,
>>                       "apr_socket_timeout_set");
>>      }
> 
> For expediency's sake, +1


+1.

-Paul

Re: svn commit: r372037 - in /httpd/httpd/branches/2.0.x: CHANGES STATUSserver/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Brad Nicholes wrote:
> Applying the following patch should fix it.
> 
> Index: core.c
> ===================================================================
> --- core.c	(revision 372564)
> +++ core.c	(working copy)
> @@ -4525,7 +4525,7 @@
>      rv = apr_socket_timeout_set(csd, c->base_server->timeout);
>      if (rv != APR_SUCCESS) {
>          /* expected cause is that the client disconnected already */
> -        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
> +        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c,
>                       "apr_socket_timeout_set");
>      }

For expediency's sake, +1

Re: svn commit: r372037 - in /httpd/httpd/branches/2.0.x: CHANGES STATUSserver/core.c

Posted by Brad Nicholes <BN...@novell.com>.
>>> On 1/26/2006 at 11:02:14 am, in message
<43...@novell.com>,
BNICHOLES@novell.com wrote:
>>>> On 1/24/2006 at 3:45:47 pm, in message
> <20...@minotaur.apache.org>, colm@apache.org 
> wrote:
>> Author: colm
>> Date: Tue Jan 24 14:45:43 2006
>> New Revision: 372037
>> 
>> URL: http://svn.apache.org/viewcvs?rev=372037&view=rev 
>> Log:
>> Backport the NET_TIME elimination fix.
>> 
>> Submitted by: wrowe
>> 
>> Modified:
>>     httpd/httpd/branches/2.0.x/CHANGES
>>     httpd/httpd/branches/2.0.x/STATUS
>>     httpd/httpd/branches/2.0.x/server/core.c
>> 
>> Modified: httpd/httpd/branches/2.0.x/server/core.c
>> URL: 
>>
>
http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/server/core.c?rev=3

> 7
> 
>> 2037&r1=372036&r2=372037&view=diff
>>
>
============================================================================
>> ==
>> --- httpd/httpd/branches/2.0.x/server/core.c (original)
>> +++ httpd/httpd/branches/2.0.x/server/core.c Tue Jan 24 14:45:43
> 2006
>> @@ -3679,11 +3679,9 @@
>>      }
>>  
>>      if (mode != AP_MODE_INIT && mode != AP_MODE_EATCRLF) {
>> -        if (ctx->first_line) {
>> -            apr_socket_timeout_set(ctx->csd, 
>> -                                   keptalive
>> -                                      ?
> f->c->base_server->keep_alive_timeout
>> -                                      :
> f->c->base_server->timeout);
>> +        if (keptalive && ctx->first_line) {
>> +            apr_socket_timeout_set(ctx->csd,
>> +                                  
> f->c->base_server->keep_alive_timeout);
>>              ctx->first_line = 0;
>>          }
>>          else {
>> @@ -4493,10 +4491,9 @@
>>  static int core_pre_connection(conn_rec *c, void *csd)
>>  {
>>      core_net_rec *net = apr_palloc(c->pool, sizeof(*net));
>> -
>> -#ifdef AP_MPM_DISABLE_NAGLE_ACCEPTED_SOCK
>>      apr_status_t rv;
>>  
>> +#ifdef AP_MPM_DISABLE_NAGLE_ACCEPTED_SOCK
>>      /* BillS says perhaps this should be moved to the MPMs. Some
> OSes
>>       * allow listening socket attributes to be inherited by the
>>       * accept sockets which means this call only needs to be made
>> @@ -4518,6 +4515,20 @@
>>                        "apr_socket_opt_set(APR_TCP_NODELAY)");
>>      }
>>  #endif
>> +
>> +    /* The core filter requires the timeout mode to be set, which
>> +     * incidentally sets the socket to be nonblocking.  If this
>> +     * is not initialized correctly, Linux - for example - will
>> +     * be initially blocking, while Solaris will be non blocking
>> +     * and any initial read will fail.
>> +     */
>> +    rv = apr_socket_timeout_set(csd, c->base_server->timeout);
>> +    if (rv != APR_SUCCESS) {
>> +        /* expected cause is that the client disconnected already
> */
>> +        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
>> +                     "apr_socket_timeout_set");
>> +    }
>> +
>>      net->c = c;
>>      net->in_ctx = NULL;
>>      net->out_ctx = NULL
> 
> This patch breaks the NetWare build with an "Illegal implicit
> conversion"  trying to pass a 'struct conn_rec *' to ap_log_error()
> rather than a 'struct server_rec *'.  Seems a little dangerous to be
> passing a pointer to a completely different structure.  Either the
calls
> to ap_log_error() should be removed or a compatible structure
pointer
> needs to be passed in.
> 
> Bra

Applying the following patch should fix it.

Index: core.c
===================================================================
--- core.c	(revision 372564)
+++ core.c	(working copy)
@@ -4525,7 +4525,7 @@
     rv = apr_socket_timeout_set(csd, c->base_server->timeout);
     if (rv != APR_SUCCESS) {
         /* expected cause is that the client disconnected already */
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c,
                      "apr_socket_timeout_set");
     }