httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@covalent.net>
Subject Re: [PATCH] Take two on the conn_rec change
Date Wed, 18 Apr 2001 03:09:49 GMT
On Tue, 17 Apr 2001, Greg Stein wrote:

> Okay... I think the patch is fine. Just nits at this point.

I'll fix most of the nits before I actually commit.

> +1 because I'd like to see more HTTP stuff move back to modules/http/. IMO,
> it was a mistake to move them back to server/ in the first place :-)

As I said when I moved it, it was a guess.  Things are likely to move back
and forth for a little while.

> On Tue, Apr 17, 2001 at 02:39:26PM -0700, rbb@covalent.net wrote:
> >...
> > +static int read_request_line(request_rec *r)
>
> Were there any changes to this function during the move? It is very
> difficult to tell from the patch.

Nothing changed, but I changed the name, which I am going to change it
back, since it is still a static function.

> >...
> > +static int http_create_request(request_rec *r)
> > +{
> > +    ap_http_conn_rec *hconn = ap_get_module_config(r->connection->conn_config,
&http_module);
> > +    unsigned keptalive;
>
> "int"

I used unsigned, because it was unsigned in the structure.

> > --- modules/http/mod_core.h	2001/02/28 15:24:07	1.6
> > +++ modules/http/mod_core.h	2001/04/17 21:29:28
> > @@ -70,6 +70,13 @@
> >   * @package mod_core private header file
> >   */
> >
> > +typedef struct ap_http_conn_rec ap_http_conn_rec;
> > +
> > +struct ap_http_conn_rec {
> > +    /** How many times have we used it? */
> > +    int keepalives;
> > +};
>
> Is it really worth it to create this structure? Other guys use it, so it
> isn't private. Do we envision more stuff going into this from the conn_rec?
> (i.e. moving soon; we can always create this structure later)

The only other file that uses this structure is mod_log_config, and that
is going to change later tonight.  I would rather use the structure,
because it makes it much easier to move stuff into this structure later.
I hope that more people start to look at the stuff in the conn_rec, and
move things out of it as soon as they can.

> > @@ -571,7 +458,9 @@
> >      r->notes           = apr_table_make(r->pool, 5);
> >
> >      r->request_config  = ap_create_request_config(r->pool);
> > -    ap_run_create_request(r);
> > +    if (ap_run_create_request(r) != 0) {
> > +        return NULL;
> > +    }
>
> That can return OK, DECLINE, or some kind of error (DONE in your patch). I
> believe that if DECLINE is returned, then we ought to error out (i.e. nobody
> is handling the request).

DECLINE means that the current module isn't handling the request, but the
server keeps calling the other modules.  At this point in the code, we can
not receive DECLINED.

Ryan

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


Mime
View raw message