The OpenSSL source doesn’t look the same as it did a year ago. Matt posted about the big code reformatting. In this post I want review some of the other changes – these rarely affect features, but are more than involved than “just” whitespace.
Previously, we’d accept platform patches from just about anyone. This led to some really hard-to-follow pre-processor guards:
#if !defined(TERMIO) && !defined(TERMIOS) \ && !defined(OPENSSL_SYS_VMS) && !defined(OPENSSL_SYS_MSDOS) \ && !defined(OPENSSL_SYS_MACINTOSH_CLASSIC) && \ !defined(MAC_OS_GUSI_SOURCE)
Can anyone reasonably tell when that applies? And there were many that were just plain silly:
#if 1 /* new with OpenSSL 0.9.7 */ #ifdef undef
So we did a couple of things.
First, we’re no longer supporting every known platform; it has to be something that either someone on the team has direct access to, or we have extensive support from the vendor.
Back in the fall we kicked off a discussion on the openssl-dev mailing list about removing unsupported platforms. Based on feedback, Netware is still supported and people are contacting some vendors to get them to step up and help support their platform. But the following are now gone:
- Sony NEWS4
- BEOS and BEOS_R5
- Sinix/ReliantUNIX RM400
OpenSSL never supported 16-bit (neither did SSLeay, really), and we just never made that explicit.
Almost every instance of
#if 0 or
#if 1 had the appropriate code
removed. There are a couple of places where they remain, because it could
be useful documentation. For example, this block in
#if 0 /* * Do not set the compare functions, because this may lead to a * reordering by "id". We want to keep the original ordering. We may pay * a price in performance during sk_SSL_CIPHER_find(), but would have to * pay with the price of sk_SSL_CIPHER_dup(). */ sk_SSL_CIPHER_set_cmp_func(srvr, ssl_cipher_ptr_id_cmp); sk_SSL_CIPHER_set_cmp_func(clnt, ssl_cipher_ptr_id_cmp); #endif
The once place they do remain is in the
crypto/bn component. That’s just
because I’m scared to touch it. :) That directory counts for about
one-third of all remaining instances. Compared to 1.0.2, which had 343
instances in 165 files, we now have 43 instances in 31 files, and I hope
we can reduce that even more.
Over time, some feature-control
#ifdef’s evolved into inconsistencies.
We cleaned them up, merging
OPENSSL_NO_FP_API was merged into
OPENSSL_NO_STDIO. (We’ll soon make that buildable again, for embedded
Also in this area, configuration used
which was internally mapped to
OPENSSL_SYS_xxx. Removing that mapping,
in conjunction with removing some old platforms, made parts of the
internal header file
e_os.h much simpler, but we there is still room
for much improvement there.
The biggest change was removing about one-third of the nearly 100 options
that we used to support. Many of these had suffered severe bit-rot and
no longer worked. The biggest part was removing the ability to build
OpenSSL without other parts of the library:
STACK are no longer optional;
OPENSSL_NO_BIO has no effect (and will hopefully generate a warning). We
also removed support for the broken SHA0 and DSS0, the CBCM mode of DES
(invented by IBM and not used anywhere), and the maintenance of our own free
OPENSSL_NO_BUF_FREELISTS. This last got us some flack erroneously
sent our way, but it was clearly a holdover from when the major supported
platforms did not have efficient
Dynamic memory cleanup
We did a big cleanup on how we use various memory functions. For example:
a = (foo *)malloc(n * sizeof(foo))
had a needless cast, and would quietly break if the type of
We now do this consistently:
a = malloc(n * sizeof(*a));
sizeof changes were done for
We also stopped checking for
NULL before calling
free. That was huge,
requiring over a dozen commits. It also reduced code complexity,
and got us a few extra percentage points on our test coverage. :)
The OpenSSL code base has a long and storied history. With these changes, the reformatting, and others, I think we’re making good on our commitment for the future.