httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@covalent.net>
Subject Re: cvs commit: apr/network_io/unix sendrecv.c
Date Mon, 26 Feb 2001 14:53:31 GMT
On Mon, 26 Feb 2001, Greg Stein wrote:

> On Mon, Feb 26, 2001 at 04:38:23AM -0000, rbb@apache.org wrote:
> > rbb         01/02/25 20:38:23
> >
> >   Modified:    .        CHANGES libhttpd.dsp
> >                build    rules.mk
> >                include  http_protocol.h
> >                modules/generators mod_asis.c mod_autoindex.c mod_cgid.c
> >                modules/http http_protocol.c mod_core.h
> >                modules/mappers mod_imap.c
> >                server   Makefile.in
> >                include  apr_general.h
> >                network_io/unix sendrecv.c
> >   Added:       server   protocol.c
> >   Log:
> >   Begin to move functions from the http module to the core.  The goal is to
> >   have only functions that are HTTP specific in the http directory.
>
> I don't understand this checkin at all...
>
> mod_core.h was to be private to the mod_core module. Now it is public, yet
> located in a different directory. If it is to be public, then it should just
> go into httpd-2.0/include. Hell, why even make it exist, given that we have
> things like http_protocol.h?
>

mod_core will become private again in time.  Right now, things are in
flux, and to keep things building I had to make it public.

> Second, I see that server/protocol.c was created and a ton of stuff pulled
> from modules/http/http_protocol.c and shoved into protocol.c. However, most
> of those functions *are* http-specific. So why'd they get moved? If the
> intent was to keep http-specific stuff in modules/http, then what's up?

Any function I moved is a function I don't believe is protocol specific at
all.  They are functions that are part of how Apache works, and to put
them in the http module means that no other protocol module can use them.
Having said that, I am perfectly willing to be wrong.  Things will likely
move back and forth until we get this right.

BTW, my goal is to compile an Apache without mod_http, but with another
protocol module, and still have it work cleanly.  Currently, that is not
possible.

> >...
> >   1.1                  httpd-2.0/server/protocol.c
> >
> >   Index: protocol.c
> >...
> >   const char *ap_make_content_type(request_rec *r, const char *type)
>
> Content-Type is HTTP-specific.

I disagree.  Content-type is a part of the request_rec, and it is integral
to how Apache works.  Keeping that in HTTP just means that every protocol
module needs to have HTTP loaded in order to work properly.

>
> >...
> >   static int parse_byterange(char *range, apr_off_t clength,
> >                              apr_off_t *start, apr_off_t *end)
> >...
>
> All the byterange stuff is HTTP-specific.

Again, I disagree.  byteranges is how Apache sends partial content.
Imagine an FTP module that wants to do a REST command.  The easiest way
(without writing a lot more code), is to send a simple byterange command
for the end part of the data.  This works because of the way that
byterange was specified if there is only one range.

> >...
> >   AP_DECLARE(void) ap_set_content_length(request_rec *r, apr_off_t clength)
> >...
> >   AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak)
> >...
> >   AP_DECLARE(void) ap_set_etag(request_rec *r)
>
> HTTP-specific.

Do not tell me that set_content_length is HTTP specific.  Almost every
protocol out there has some sort of content-length.  Soemthing where both
sides need to know how much was sent.  The Etag stuff I went back and
forth over.  I am 100% willing to put it back in HTTP.

>
> >...
> >   AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
>
> HTTP-specific. This understands the HTTP header/body mechanism.

Take a second look please.  This does header-folding, and it makes sure
that all of the data has the same format (data\r).  I can see no place in
this code where it knows anything about HTTP specific stuff.  Not saying
this function will be used by all protocol modules, but it should be used
by a lot of them.

>
> >...
> >   AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri)
> >...
> >   static int read_request_line(request_rec *r)
> >...
> >   static void get_mime_headers(request_rec *r)
>
> HTTP-specific.

Again, I disagree (obviously, or I wouldn't have moved them).  Apache
works based on URI's.  Also, URI's shouldn't be protocol specific, why is
parsing them?  read_request_line, that one you are probably right, it
should go back into http_protocol, same for get_mime_headers.

> >...
> >   request_rec *ap_read_request(conn_rec *conn)
>
> Tough one. This is HTTP specific today, so I'd probably say leave it in
> modules/http and create a new, generic request builder in server/protocol.c.

read_request just fills out a request_rec.  How is that HTTP specific
(other than that the request_rec is HTTP specific right now).  Every
protocol will need a way to fill out that structure.

> >...
> >   void ap_set_sub_req_protocol(request_rec *rnew, const request_rec *r)
>
> Urf. Probably HTTP specific, too.

All of the sub_request stuff should be in the core.  They are part of how
Apache works.  They look HTTP specific, because the request_rec is HTTP
specific (more on that later this week BTW), but sub_requests are integral
to Apache itself.

> >...
> >   AP_DECLARE(void) ap_note_auth_failure(request_rec *r)
> >...
> >   AP_DECLARE(void) ap_note_basic_auth_failure(request_rec *r)
> >...
> >   AP_DECLARE(void) ap_note_digest_auth_failure(request_rec *r)
> >...
> >   AP_DECLARE(int) ap_get_basic_auth_pw(request_rec *r, const char **pw)
>
> All HTTP specific.

I looked very hard at these.  I couldn't really decide, and they are more
about authentication than anything.  I figured we could easily make them
generic, so I moved them.

>
> >...
> >   AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_length_filter(ap_filter_t *f,
> >                                                                 apr_bucket_brigade
*b)
> >...
> >   static int ap_set_byterange(request_rec *r)
>
> HTTP specific again.

The content_length filter is NOT HTTP specific.  It records how much body
data was sent.  This is information ANY protocol will need, if for no
other reason than for logging, which is what we REALLY use it for.
set_byterange is needed for the byterange filter, which as I said before
is not HTTP specific.

> I'm obviously really confused. What is your plan? Are all these HTTP
> functions there only temporarily? I just don't understand...

All these functions are moved because they either are now, or will be soon
not HTTP specific.  They are protocol functions that MOST, not all, but
most, protocols will need to work cleanly.

This stuff is in flux, and it will be ugly for a little while, be patient.
This is a big job separating the HTTP specific from the generic, and I am
likely to get a few wrong.  When I do, let's talk about them, and then
move them back if we all decide to.  I am looking at each function
individually, and making decisions based on what I know of HTTP, FTP,
and POP.  This isn't a wide enough sample, but I am more likely to miss
some because my protocol list is too short than to move too many
functions.

Ryan

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


Mime
View raw message