Return-Path: Delivered-To: apmail-apache-cvs-archive@apache.org Received: (qmail 60914 invoked by uid 500); 27 May 2000 20:53:27 -0000 Mailing-List: contact apache-cvs-help@apache.org; run by ezmlm Precedence: bulk X-No-Archive: yes Reply-To: new-httpd@apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list apache-cvs@apache.org Received: (qmail 60888 invoked by uid 500); 27 May 2000 20:53:26 -0000 Delivered-To: apmail-apache-2.0-cvs@apache.org X-Authentication-Warning: koj.rkbloom.net: rbb owned process doing -bs Date: Fri, 26 May 2000 13:52:50 -0700 (PDT) From: rbb@covalent.net X-Sender: rbb@koj.rkbloom.net To: new-httpd@apache.org cc: apache-2.0-cvs@apache.org Subject: Re: cvs commit: apache-2.0/src/modules/standard mod_access.c mod_alias.c mod_asis.c mod_auth.c mod_auth_anon.c mod_auth_digest.c mod_autoindex.c mod_cern_meta.c mod_cgi.c mod_cgid.c mod_dir.c mod_echo.c mod_env.c mod_expires.c mod_file_cache.c mod_headers.c mod_imap.c mod_include.c mod_info.c mod_log_config.c mod_mime.c mod_negotiation.c mod_rewrite.c mod_setenvif.c mod_so.c mod_speling.c mod_status.c mod_unique_id.c mod_usertrack.c mod_vhost_alias.c In-Reply-To: <20000527052808.13588.qmail@locus.apache.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Rating: locus.apache.org 1.6.2 0/1000/N I have a bunch of issues with this patch. On 27 May 2000 wrowe@locus.apache.org wrote: > This patch corrects the issues from the AP_EXPORT and linkage > specification arguments to the ap_hooks.h declarations. As with > the APR_ and AP_ patches, API_VAR_EXPORT becomes API_EXPORT_VAR, > and MODULE_VAR_EXPORT becomes MODULE_EXPORT_VAR. Why? This is a name change, and a gratuitous one at that. The name change made sense in APR, because APR is a separate project. Here, it just becomes a big commit and breaks a lot of knowledge people used to have. > I will be happy to revert the inclusion of ap_config.h from > httpd.h if this bothers anyone. More individual modules need > to be patched if we do so. There was a reason that this was changed originally. I don't remember what it was, but I suggest looking back at the archives. > This patch also moves the following data from http_main to http_config: > > const char *ap_server_argv0; > const char *ap_server_root; > ap_array_header_t *ap_server_pre_read_config; > ap_array_header_t *ap_server_post_read_config; > ap_array_header_t *ap_server_config_defines; Why? > /* Hooks */ > -AP_DECLARE_HOOK(int,header_parser,(request_rec *)) > -AP_DECLARE_HOOK(void,post_config, > +AP_DECLARE_HOOK(API_EXPORT,int,header_parser,(request_rec *)) > +AP_DECLARE_HOOK(API_EXPORT,void,post_config, > (ap_pool_t *pconf,ap_pool_t *plog,ap_pool_t *ptemp,server_rec *s)) > -AP_DECLARE_HOOK(void,open_logs, > +AP_DECLARE_HOOK(API_EXPORT,void,open_logs, > (ap_pool_t *pconf,ap_pool_t *plog,ap_pool_t *ptemp,server_rec *s)) > -AP_DECLARE_HOOK(void,child_init,(ap_pool_t *pchild, server_rec *s)) > +AP_DECLARE_HOOK(API_EXPORT,void,child_init,(ap_pool_t *pchild, server_rec *s)) Every single hook is being declared API_EXPORT. This seems silly to me. Can't we just use API_EXPORT in the macro definition? > -#ifndef MODULE_VAR_EXPORT > -#define MODULE_VAR_EXPORT > +#ifndef MODULE_EXPORT_VAR > +#define MODULE_EXPORT_VAR > #endif > -#ifndef API_VAR_EXPORT > -#define API_VAR_EXPORT > +#ifndef API_EXPORT_VAR > +#define API_EXPORT_VAR > #endif These are name changes for the sake of name changes, blech. I am basically -1 for this whole commit unless somebody can explain why the name change was necessary. I would really like to see this whole thing removed, and then possibly re-committed in small pieces that are much easier to review. Ryan