You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2004/11/10 16:11:23 UTC

cvs commit: httpd-2.0/modules/ssl config.m4

jorton      2004/11/10 07:11:23

  Modified:    modules/ssl config.m4
  Log:
  * modules/ssl/config.m4: Use libtool's -export-symbols-regex flag to
  hide all global symbols defined by mod_ssl other than the module
  structure (where possible).
  
  Revision  Changes    Path
  1.24      +5 -0      httpd-2.0/modules/ssl/config.m4
  
  Index: config.m4
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/config.m4,v
  retrieving revision 1.23
  retrieving revision 1.24
  diff -d -w -u -r1.23 -r1.24
  --- config.m4	6 Mar 2004 16:47:41 -0000	1.23
  +++ config.m4	10 Nov 2004 15:11:23 -0000	1.24
  @@ -116,6 +116,11 @@
       APACHE_CHECK_SSL_TOOLKIT
       APR_SETVAR(MOD_SSL_LDADD, [\$(SSL_LIBS)])
       CHECK_DISTCACHE
  +    if test "x$enable_ssl" = "xshared"; then
  +       # The only symbol which needs to be exported is the module
  +       # structure, so ask libtool to hide everything else:
  +       APR_ADDTO(MOD_SSL_LDADD, [-export-symbols-regex ssl_module])
  +    fi
   ])
   
   # Ensure that other modules can pick up mod_ssl.h
  
  
  

Re: cvs commit: httpd-2.0/modules/ssl config.m4

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Nov 10, 2004 at 10:38:23PM -0600, William Rowe wrote:
> It's the 80/20 solution, 80%+ of other modules never need to see
> the cruft, especially the cruft related to the underlying ssl 
> library.  

I don't think anything should be exposed by mod_ssl.h other than
optional function hooks to avoid creating tight coupling between modules
- that's one of 2.0's selling points.  If you think more hooks are
needed, of course, that shouldn't be an issue.

> >> Aside from this, the solution is unlikely to be truly portable.
> >
> >It's a libtool flag and libtool either supports or ignores it.
> 
> Compilers can't ignore the static fn() construct; they are always
> explicitly private.  By far a better solution.

Making everything static would mean moving the entire module into a
single C file, I can't see how that's really viable.

> Could you describe what this patch accomplishes or was ment
> to accomplish?

It prevents global symbol table pollution (the use of {SSL,ssl}_* is
hardly unique to mod_ssl), it forces other modules to DTRT and use the
optional hooks rather than e.g. use ssl_var_lookup() directly, and it
makes function calls within mod_ssl fractionally faster: nothing that
blows you away but it's free for a single libtool flag.

Regards,

joe


Re: cvs commit: httpd-2.0/modules/ssl config.m4

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 01:03 PM 11/10/2004, Joe Orton wrote:
>On Wed, Nov 10, 2004 at 10:54:57AM -0600, William Rowe wrote:
>> Sorry, veto on this one.  I actually have a module that uses
>> mod_ssl's implementation to inject an apache ssl filter for 
>> an ftp datastream.  Please, revert.
>
>I guess your module has been broken against HEAD since mod_ssl.h stopped
>exposing mod_ssl internals, so that's not good justification to revert
>this.  You can't get to mod_ssl internal functions at compile time
>already: this just hides them at link time too.

Quite probable... it's built for 2.0.x / apr 0.9.  I hope to start
testing after AC on 2.1 / APR 1.0.  I totally agree with keeping
the internal functions out of mod_ssl.h - this header should have
almost nothing to do with the implementation.

It's the 80/20 solution, 80%+ of other modules never need to see
the cruft, especially the cruft related to the underlying ssl 
library.  

>> Aside from this, the solution is unlikely to be truly portable.
>
>It's a libtool flag and libtool either supports or ignores it.

Compilers can't ignore the static fn() construct; they are always
explicitly private.  By far a better solution.

Could you describe what this patch accomplishes or was ment
to accomplish?

Bill


Re: cvs commit: httpd-2.0/modules/ssl config.m4

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Nov 10, 2004 at 10:54:57AM -0600, William Rowe wrote:
> Sorry, veto on this one.  I actually have a module that uses
> mod_ssl's implementation to inject an apache ssl filter for 
> an ftp datastream.  Please, revert.

I guess your module has been broken against HEAD since mod_ssl.h stopped
exposing mod_ssl internals, so that's not good justification to revert
this.  You can't get to mod_ssl internal functions at compile time
already: this just hides them at link time too.

> Aside from this, the solution is unlikely to be truly portable.

It's a libtool flag and libtool either supports or ignores it.

joe

Re: cvs commit: httpd-2.0/modules/ssl config.m4

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Sorry, veto on this one.  I actually have a module that uses
mod_ssl's implementation to inject an apache ssl filter for 
an ftp datastream.  Please, revert.

Aside from this, the solution is unlikely to be truly portable.
A better solution would be a more aggressive use of static fn()
declarations (which are portable, by definition.)  

What I -think- you are looking for is an option to apr_dso_load
which would load a library/module, without our default of all
public symbols.  The apr use of the RTLD_GLOBAL flag is overkill 
99% of the time for httpd modules, and this could also be made 
an option.  For those modules which need to be public, adding 
a third argument of public | private to the LoadModule directive 
could provide the functionality.

Bill

At 09:11 AM 11/10/2004, you wrote:
>jorton      2004/11/10 07:11:23
>
>  Modified:    modules/ssl config.m4
>  Log:
>  * modules/ssl/config.m4: Use libtool's -export-symbols-regex flag to
>  hide all global symbols defined by mod_ssl other than the module
>  structure (where possible).
>  
>  Revision  Changes    Path
>  1.24      +5 -0      httpd-2.0/modules/ssl/config.m4
>  
>  Index: config.m4
>  ===================================================================
>  RCS file: /home/cvs/httpd-2.0/modules/ssl/config.m4,v
>  retrieving revision 1.23
>  retrieving revision 1.24
>  diff -d -w -u -r1.23 -r1.24
>  --- config.m4 6 Mar 2004 16:47:41 -0000       1.23
>  +++ config.m4 10 Nov 2004 15:11:23 -0000      1.24
>  @@ -116,6 +116,11 @@
>       APACHE_CHECK_SSL_TOOLKIT
>       APR_SETVAR(MOD_SSL_LDADD, [\$(SSL_LIBS)])
>       CHECK_DISTCACHE
>  +    if test "x$enable_ssl" = "xshared"; then
>  +       # The only symbol which needs to be exported is the module
>  +       # structure, so ask libtool to hide everything else:
>  +       APR_ADDTO(MOD_SSL_LDADD, [-export-symbols-regex ssl_module])
>  +    fi
>   ])
>   
>   # Ensure that other modules can pick up mod_ssl.h
>  
>  
>