You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@ibm.net> on 2000/07/06 17:25:52 UTC

Re: cvs commit: apache-2.0/src/modules/standard mod_file_cache.c

> Date: Thu, 6 Jul 2000 08:13:32 -0700 (PDT)
> From: Bill Stoddard <st...@locus.apache.org>
> 
>   Index: apr_network_io.h
>   ===================================================================
>   RCS file: /home/cvs/apache-2.0/src/lib/apr/include/apr_network_io.h,v
>   retrieving revision 1.44
>   retrieving revision 1.45
>   diff -u -r1.44 -r1.45
>   --- apr_network_io.h	2000/06/29 14:41:22	1.44
>   +++ apr_network_io.h	2000/07/06 15:13:22	1.45
>   @@ -87,6 +87,7 @@
>    #define APR_SO_TIMEOUT       32
>    #define APR_SO_SNDBUF        64
>    #define APR_SO_RCVBUF        128
>   +#define APR_SO_DISCONNECTED  256
>    
>    #define APR_POLLIN    0x001 
>    #define APR_POLLPRI   0x002
>   @@ -358,7 +359,7 @@
>        arg 3) A structure containing the headers and trailers to send
>        arg 4) Offset into the file where we should begin writing
>        arg 5) Number of bytes to send 
>   -    arg 6) OS-specific flags to pass to sendfile()
>   +    arg 6) APR flags that are mapped to OS specific flags

I'm confused...  Where is APR_SENDFILE_DISCONNECT_SOCKET defined?

>    
>    B<NOTE>:  This functions acts like a blocking write by default.  To change 
>              this behavior, use ap_setsocketopt with the APR_SO_TIMEOUT option.
>   @@ -440,6 +441,8 @@
>                                        don't wait at all.
>                  APR_SO_SNDBUF     --  Set the SendBufferSize
>                  APR_SO_RCVBUF     --  Set the ReceiveBufferSize
>   +              APR_SO_DISCONNECTED -- Query the disconnected state of the socket.
>   +                                    (Currently only used on
>    Windows)

If I can pass in the flag on any platform, don't I need to be able to
query the socket option value on any platform?  I guess this isn't an
issue because the WinNT MPM is the only code to query the option value.

>   Index: mod_file_cache.c
>   ===================================================================
>   RCS file: /home/cvs/apache-2.0/src/modules/standard/mod_file_cache.c,v
>   retrieving revision 1.13
>   retrieving revision 1.14
>   diff -u -r1.13 -r1.14
>   --- mod_file_cache.c	2000/07/05 18:52:57	1.13
>   +++ mod_file_cache.c	2000/07/06 15:13:30	1.14
>   @@ -421,7 +421,15 @@
>        struct iovec iov;
>        ap_hdtr_t hdtr;
>        ap_hdtr_t *phdtr = &hdtr;
>   +    ap_int32_t flags = 0;
>    
>   +    if (!r->connection->keepalive) {
>   +        /* Prepare the socket to be reused. Ignored on systems
>   +         * that do not support reusing the accept socket
>   +         */
>   +        flags |= APR_SENDFILE_DISCONNECT_SOCKET;
>   +    }
>   +
>        /* 
>         * We want to send any data held in the client buffer on the
>         * call to iol_sendfile. So hijack it then set outcnt to 0
>   @@ -446,7 +454,7 @@
>                         phdtr,
>                         &offset,
>                         &length,
>   -                     0);
>   +                     flags);
>        }
>        else {
>            while (ap_each_byterange(r, &offset, &length)) {
>   @@ -455,7 +463,7 @@
>                             phdtr,
>                             &offset,
>                             &length,
>   -                         0);
>   +                         flags);
>                phdtr = NULL;
>            }
>        }

Ummm.... this call is in a loop for each byterange... you only want to
enable APR_SENDFILE_DISCONNECT_SOCKET on the last call in the loop,
right? 


-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Re: cvs commit: apache-2.0/src/modules/standard mod_file_cache.c

Posted by Bill Stoddard <st...@raleigh.ibm.com>.
> >
> >    #define APR_POLLIN    0x001
> >    #define APR_POLLPRI   0x002
> >   @@ -358,7 +359,7 @@
> >        arg 3) A structure containing the headers and trailers to send
> >        arg 4) Offset into the file where we should begin writing
> >        arg 5) Number of bytes to send
> >   -    arg 6) OS-specific flags to pass to sendfile()
> >   +    arg 6) APR flags that are mapped to OS specific flags
>
> I'm confused...  Where is APR_SENDFILE_DISCONNECT_SOCKET defined?
>

apr/include/apr_network_io.h

> >
> >    B<NOTE>:  This functions acts like a blocking write by default.  To change
> >              this behavior, use ap_setsocketopt with the APR_SO_TIMEOUT option.
> >   @@ -440,6 +441,8 @@
> >                                        don't wait at all.
> >                  APR_SO_SNDBUF     --  Set the SendBufferSize
> >                  APR_SO_RCVBUF     --  Set the ReceiveBufferSize
> >   +              APR_SO_DISCONNECTED -- Query the disconnected state of the socket.
> >   +                                    (Currently only used on
> >    Windows)
>
> If I can pass in the flag on any platform, don't I need to be able to
> query the socket option value on any platform?  I guess this isn't an
> issue because the WinNT MPM is the only code to query the option value.
>

I considered this case but didn't follow through as throughly as perhaps I should have. I
think you will get
APR_EINVAL if you try to query APR_SO_DISCONNECTED on Unix. Since we do not check
return codes on ap_getsockopt (that I am aware of anyway) this shouldn't be a problem.
Furthermore,
we only check for APR_SO_DISCONNECTED in the Windows MPM. Feel free to hack at it more.

> >        else {
> >            while (ap_each_byterange(r, &offset, &length)) {
> >   @@ -455,7 +463,7 @@
> >                             phdtr,
> >                             &offset,
> >                             &length,
> >   -                         0);
> >   +                         flags);
> >                phdtr = NULL;
> >            }
> >        }
>
> Ummm.... this call is in a loop for each byterange... you only want to
> enable APR_SENDFILE_DISCONNECT_SOCKET on the last call in the loop,
> right?


Good catch! The wonders of peer review!  Fixing it now.

Bill


Re: cvs commit: apache-2.0/src/modules/standard mod_file_cache.c

Posted by rb...@covalent.net.
> >    
> >    B<NOTE>:  This functions acts like a blocking write by default.  To change 
> >              this behavior, use ap_setsocketopt with the APR_SO_TIMEOUT option.
> >   @@ -440,6 +441,8 @@
> >                                        don't wait at all.
> >                  APR_SO_SNDBUF     --  Set the SendBufferSize
> >                  APR_SO_RCVBUF     --  Set the ReceiveBufferSize
> >   +              APR_SO_DISCONNECTED -- Query the disconnected state of the socket.
> >   +                                    (Currently only used on
> >    Windows)
> 
> If I can pass in the flag on any platform, don't I need to be able to
> query the socket option value on any platform?  I guess this isn't an
> issue because the WinNT MPM is the only code to query the option value.

Yes, this option needs to be query-able on ALL platforms.  Ignore the fact
that only the Windows MPM is using this code.  You are in APR now, and
platforms don't matter when it comes to interfaces.  The only difference
most platforms should see with regard to APR, is that some functions
either aren't defined or they return APR_ENOTIMPL.  No argument should be
specific to a specific platform.

Ryan
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------