httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1)" <madhusudan_mathiha...@hp.com>
Subject RE: [PATCH] openssl configuration (v2)
Date Wed, 12 Mar 2003 22:13:00 GMT

I tried the patch, and it seemed to work fine for me (and it's more cleaner
than what we have today).

- I don't know how SSL-C will be broken with this patch
- Anybody out there using SSL-C ?
- I have a patch to get SSL-C to work with mod_ssl.. I'll have to dust it
out, before posting the patch.
- any objections

-Madhu

-----Original Message-----
From: Geoff Thorpe [mailto:geoff@geoffthorpe.net]
Sent: Friday, March 07, 2003 12:59 PM
To: dev@httpd.apache.org
Cc: Justin Erenkrantz
Subject: Re: [PATCH] openssl configuration (v2)


Hi there,

* Justin Erenkrantz (justin@erenkrantz.com) wrote:
> 
> Yes, the former is what I would expect as well.  I would add a third
clause 
> to your AC_CHECK_LIB which appends -lLIBRARY to saved_LIBS.  Hmm.  I
wonder 
> where saved_LIBS will end up in relationship to AP_LIBS.  Ah, it seems
that 
> EXTRA_LIBS will be before AP_LIBS - Solaris and other one-pass linkers 
> should be happy.

Yeah I've changed this to manually add "-lssl -lcrypto" to LIBS after
it's restored from saved_LIBS and that makes things work. I also added a
missing AC_MSG_ERROR() if the libs weren't found, as this is clearly a
fatal SSL/TLS configuration error I should have been catching already.

So I guess we make do with that then? I know this is what is currently
done so I presume this is OK, though I admit to being less than pleased
with manually added linker flags given that we're using AC_CHECK_LIB()
which is supposed to hide all that (and handle whatever the correct
syntax is on the host).

NB: If it's later decided that AC_CHECK_LIB() should take care of adding
the linker flags internally, the save/restore trick with LIBS would have
to go, which in turn means that the other linker flags would have to be
configured in advance using AC_CHECK_LIB macros (rather than using
$apr_config --libs).

> >in portability problems. Unless someone tells me the Apache-Approved(tm)
> >way to do this, I'd rather just leave the comment there to guide someone
> >else if they feel so moved afterwards.
> 
> ap_ssltk_version="`$p/openssl version`"
> 
> I kinda like that approach (i.e. what we're currently doing...).

I'm not sure I like it but I'll go with the flow if you insist. The
thing is, it requires the openssl executable be installed which on
package-management systems will be the openssl package containing
programs, documents, scripts, certificates, etc. rather than just a
"libopenssl" package contains libs and headers. This also requires us to
test for the "openssl" program and work around the case that it's not
found (the current code seems to consider this fatal but that seems
dramatic under the circumstances). At the least, can we leave that as a
second step once the configuration is tidied up? Right now the version
check in configure is OK even if there's no obvious way to propogate a
text form of the version to the console. I can't see the implications of
requiring the presence of the openssl binary to be worth the effort for
a purely aesthetic issue.

> If you want to be cute, you could do something with AC_EGREP_HEADER, but 
> I'm not totally clear what the syntax would be.

I took a look because this idea seemed promising at first look, but the
code generated by AC_EGREP_HEADER sends both stdout and stderr to
/dev/null, so the regexp seems useful only for a yes/no pattern-match -
I can't see any obvious way to capture what comes out of the egrep.
Perhaps there's another macro lurking somewhere that would solve all
this?

> >- Seeing as CPPFLAGS seems ideal for header *and* compilation checks, is
> >  INCLUDES still the appropriate place to APR_ADDTO() any required
> >  include path once the tests are done?
> 
> Yeah, I thought about that after I sent my original message.  We really 
> should be adding the -I's to INCLUDES.  But, autoconf will only
temporarily 
> use CPPFLAGS, so we should add to CPPFLAGS for the header checks, then if 
> it works, add it to INCLUDES (which gets morphed to EXTRA_INCLUDES later 
> on). Our build system should use INCLUDES properly.  -- justin

OK, I'll stick with INCLUDES then. So AFAICS, if we accept the
following;

(1) explicitly adding "-lssl -lcrypto" into LIBS if the library checks
    succeed (using APR_ADDTO),
(2) don't try to grep the version text yet for output from "configure"
    nor assume the "openssl" binary to generate it,
(3) put include paths into INCLUDES (using APR_ADDTO)

then the patch attached to this mail should be OK? Does this seem
reasonable? (Note, I've still left the old version stubbed in - I prefer
to leave the diff readable until someone's ready to start using the word
"commit").

Cheers,
Geoff

-- 
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/


Mime
View raw message