>
> I am 100% against holding up a beta for this stuff. If we can keep the
> patches incredibly small and reviewable, then there is no reason that they
> have to destabilize the server.
>
I agree that this work should not be allowed to hold up the beta (and eventual
release) of Apache
2.0. One possible exception would be if the changes affect the external API in
significant ways,
which is not the case AFAIK. But I also agree that the changes can be phased in in
small easily
reviewable chunks. I am not against working incrementally to continuing to clean-up
the parts of
Apache 2.0 that hinder multiprotocol use.
> As for being blunt, isn't that why we all get along? If anybody on this
> list doesn't think they can speak their minds freely and bluntly, then
> things just don't work. :-)
>
> Ryan
>
> On Tue, 17 Apr 2001, David Reid wrote:
>
> > Have we agreed that the multi-protocol stuff that was outlined (and this
> > patch is based on) is the correct way to go? I'm not convinced that it is.
> > I REALLY dislike the fact that we appear to be pulling in 2 directions at
> > the moment.
> >
> > One group is trying to get 2.0 bug fixed and ready for release, the other
> > trying to destabilize it and insert new potential for bugs and holes!! If
> > the multi-protocol stuff is a *MUST* have for a 2.0 release then we should
> > have some real discussions and stop worrying about rolling "beta's".
> >
> > If it's not a must have then perhaps we should stop worrying about this and
> > make it an issue for 2.1? Sorry to be blunt.
> >
> > david
> > ----- Original Message -----
> > From: <[EMAIL PROTECTED]>
> > To: <[EMAIL PROTECTED]>
> > Sent: Tuesday, April 17, 2001 4:34 PM
> > Subject: Re: Multi-protocol patch, part 1
> >
> >
> > >
> > > > It no workee. (timeout handling; see below)
> > >
> > > Argh! I fixed this once a long time ago, but now that fix is gone in both
> > > of my trees. Looking at it again. :-(
> > >
> > > > > +static int http_create_request(request_rec *r)
> > > > > +{
> > > > > + http_conn_rec *conn =
> > ap_get_module_config(r->connection->conn_config, &http_module);
> > > >
> > > > Nit. Please use "hconn" to distinguish this from the "old" conn. It's
> > hard
> > > > to read, when I keep thing conn_rec.
> > >
> > > done
> > >
> > > > > + conn = apr_palloc(r->pool, sizeof(*conn));
> > > >
> > > > Maybe use pcalloc here? Certainly, for future-proofing.
> > >
> > > done
> > >
> > > > Take a look at server/protocol.c, where this stuff came from. In between
> > the
> > > > two sets of the timeout, you'll see it goes to the socket to read the
> > > > request line. Basically, it sets the timeout to <something>, reads a
> > line,
> > > > then resets it to <something>.
> > > >
> > > > In the above code, it simply sets it a couple times and exits. Not very
> > > > effective :-)
> > >
> > > I'm fixing this now. I'll repost when I am done. This is part of the
> > > problem with splitting a large patch into a couple of small patches. Not
> > > that I am convinced it works correctly in the large patch, but I know I
> > > tackled this exact bug back when I first wrote the large patch. Expect
> > > the new patch later today.
> > >
> > > > >...
> > > > > --- modules/http/mod_core.h 2001/02/28 15:24:07 1.6
> > > > > +++ modules/http/mod_core.h 2001/04/17 03:53:23
> > > >
> > > > Hrm. It would be nice to rename this to mod_http.h before we get too
> > > > attached to the mod_core.h name. (totally unrelated to this patch, but
> > just
> > > > raising it while I'm thinking of it)
> > >
> > > I agree 100%
> > >
> > > > > +typedef struct http_conn_rec http_conn_rec;
> > > >
> > > > If you're introducing a new structure, then please namespace-protect it.
> > No
> > > > sense digging a deeper hole :-)
> > >
> > > I had kind of thought that just namespace protecting it with http_ was
> > > okay. However, now that you mention it, I should use ap_http_, so I have
> > > done that now.
> > >
> > > > However, please note that we can/should simply toss the darned bit
> > fields
> > > > from conn_rec, and certainly this one.
> > >
> > > I seriously disagree with this. bit fields are incredibly useful, and we
> > > should take advantage of them, not dump them.
> > >
> > > > > #if APR_HAVE_UNISTD_H
> > > > > #include <unistd.h>
> > > > > @@ -527,13 +528,19 @@
> > > > > }
> > > > > static const char *log_connection_status(request_rec *r, char *a)
> > > > > {
> > > > > +#ifdef AP_HTTP_ENABLED
> > > > > + http_conn_rec *hconn =
> > ap_get_module_config(r->connection->conn_config,
> > > > > + &http_module);
> > > > > +#endif
> > > >
> > > > Oof! It would be nice to keep the http_conn_rec private. This kinda
> > sucks.
> > >
> > > I disagree, this REALLY sucks. :-) I think I know how to fix it, but it
> > > is a relatively large change, so I really don't want to make it here.
> > > What I believe we should basically do, is register an optional function
> > > that takes a format specifier and a function to call. Basically, the same
> > > thing we do for mod_include and mod_cgi. This is a much more extensible
> > > option.
> > >
> > > > > r->connection = conn;
> > > > > r->server = conn->base_server;
> > > > >
> > > > > - conn->keptalive = conn->keepalive == 1;
> > > >
> > > > Okay... about conn->keptalive.
> > >
> > > finally. :-)
> > >
> > > > conn_rec, but used *only* in this function.
> > > >
> > > > Rather than shift it from conn_rec to http_conn_rec, let's just switch
> > it to
> > > > a local variable here(!)
> > >
> > > Done!!!! Great Catch Greg!
> > >
> > > Ryan
> > >
> > >
> > ____________________________________________________________________________
> > ___
> > > Ryan Bloom [EMAIL PROTECTED]
> > > 406 29th St.
> > > San Francisco, CA 94131
> > > --------------------------------------------------------------------------
> > -----
> > >
> >
> >
>
>
> _______________________________________________________________________________
> Ryan Bloom [EMAIL PROTECTED]
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
>
