httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@lnd.com>
Subject RE: cvs commit: apache-2.0/src/modules/standard mod_access.cmod_alias.c mod_asis.c mod_auth.c mod_auth_anon.c mod_auth_digest.cmod_autoindex.c mod_cern_meta.c mod_cgi.c mod_cgid.c mod_dir.c mod_echo.cmod_env.c mod_expires.c mod_file_cache.c mod_headers.c
Date Sat, 27 May 2000 21:41:49 GMT
> On Fri, 26 May 2000 rbb@covalent.net wrote:
> >...
> > On 27 May 2000 wrowe@locus.apache.org wrote:
> >...
> > >      /* 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?
> 
> That's what I said, too. Consider me a "-1" on this part of 
> the commit.
> Until we have a *need* for a different linkage, this 
> parameter should not
> be included. *When* that need arises, then we can add a second set of
> macros (but leave these alone!).

No... AP is a library that is not dependent on the apache core server.
API_EXPORT_bleh are defined in the core server, but the library exists
before the core.  I've had more problems than I care for trying to take
these eggs away from the chicken.  Since *today* it's a library function,
it shouldn'd be defined in terms of the server.

If ap_hooks.c is moved to main and is compiled into the core, then I 
don't care, it becomes an argument of 'when' there is a need, and since
I have enough nays, I'll reverse those args out.

> > >   -#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.
> 
> Agreed.

As for EXPORT_VAR vs. VAR_EXPORT, it is in the schema of module_EXPORT, 
and happens to be data instead of a function.  Easier to GREP.  But if
I get the nays I'll flip *_EXPORT_VAR back to *_VAR_EXPORT under all
of APR, AP, and API.  I don't care enough to argue this point.

> If we're going to change the names, then it really should be 
> to something
> like:
> 
>   AP_MODULE_EXPORT_VAR
>   AP_EXPART_VAR
> 
> i.e. namespace protection

API_ was 'because it's there', and was the path of least resistance
(fewest changes).  APR_ because that was intuitive.  AP_ because 
that's intuitive as well, in that it is the ap library.  What prefix 
would you propose for:

  The APR library package ________
  The AP library package  ________
  The server core binary  ________
  
I think we have been through the names a dozen some times.  
As my comment in ap_config, I'm not at all happy with MODULE_squat,
and CORE_bleh is a noop as well (documentation convention?) so I 
will be looking at something better.  But not at this instant.

Now that the packages are isolated, Win32 won't suffer import/export
resolution faults, the compiler can resolve calls as jump indirects
vs. linker fixups, and no data GP faults when I bind http_main as a 
seperate binary (the Win32 Apache.exe).

> > I am basically -1 for this whole commit unless somebody can 
> > explain why the name change was necessary.
> 
> I certainly can't explain...

Which name change?  VAR_EXPORT <-> EXPORT_VAR?  AP_ vs API_ vs APR_ ?  



Mime
View raw message