httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] Take two on the conn_rec change
Date Wed, 18 Apr 2001 09:01:25 GMT
On Tue, Apr 17, 2001 at 08:09:49PM -0700, rbb@covalent.net wrote:
> On Tue, 17 Apr 2001, Greg Stein wrote:
>...
> > +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.

Yup. That wasn't a problem from my standpoint, but just describing my reason
for a +1, while we're all talking about destabilizing :-).

>...
> > > +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.

Hmm. All righty. I took a look, and another easy call about be
"vhost_lookup_data". "remain" shouldn't even exist :-). The filters and
notes could probably go, but they're a bit more difficult to justify.

Not really related to the move:

Stuff like the remote_ip and _host are probably derivatives of remote_addr,
right? Same for local_ip and _host. Although the latter is kind of
surprising since the base_server probably has that info.


> > > @@ -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.

Sure. I meant that if all modules DECLINEd, then you'd have to punt. (which
is exactly what happens now)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message