httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
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
Date Fri, 26 May 2000 20:52:50 GMT

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


Mime
View raw message