On Fri, 22 May 2015, wm4 wrote:
Move the OpenSSL and GnuTLS implementations to their own files. Other
than the connection code (including options) and some boilerplate, no
code is actually shared.
Well, the read/write/close functions are (still) pretty much identical
with just one single function call renamed, but yes, that's not enough for
keeping things more entangled than necessary.
---
There is a minor change in behavior: now the equivalent of TLS_shutdown
is called both on normal closing, and closing on the error path. I'm
not sure if this matters.
It might be worthwhile to test that it doesn't cause any direct issues
(e.g. testing with valgrind, if an error can be triggered after c->session
or c->ssl has been set).
Also, this assumes lu_zeros patch to make GnuTLS and OpenSSL exclusive
has been applied.
---
libavformat/Makefile | 4 +-
libavformat/tls.c | 427 ----------------------------------------------
libavformat/tls_common.c | 80 +++++++++
libavformat/tls_common.h | 51 ++++++
libavformat/tls_gnutls.c | 226 ++++++++++++++++++++++++
libavformat/tls_openssl.c | 233 +++++++++++++++++++++++++
6 files changed, 593 insertions(+), 428 deletions(-)
delete mode 100644 libavformat/tls.c
create mode 100644 libavformat/tls_common.c
create mode 100644 libavformat/tls_common.h
create mode 100644 libavformat/tls_gnutls.c
create mode 100644 libavformat/tls_openssl.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 73b9f63..f580d75 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -372,6 +372,8 @@ OBJS-$(CONFIG_YUV4MPEGPIPE_DEMUXER) += yuv4mpegdec.o
# external libraries
OBJS-$(CONFIG_LIBRTMP) += librtmp.o
+OBJS-$(CONFIG_GNUTLS) += tls_gnutls.o
+OBJS-$(CONFIG_OPENSSL) += tls_openssl.o
As James commented, these could be e.g. TLS-OBJS-$(CONFIG), and add
$(TLS-OBJS-YES) to the list of objects for the tls protocol.
# protocols I/O
OBJS-$(CONFIG_APPLEHTTP_PROTOCOL) += hlsproto.o
@@ -400,7 +402,7 @@ OBJS-$(CONFIG_RTP_PROTOCOL) += rtpproto.o
OBJS-$(CONFIG_SCTP_PROTOCOL) += sctp.o
OBJS-$(CONFIG_SRTP_PROTOCOL) += srtpproto.o srtp.o
OBJS-$(CONFIG_TCP_PROTOCOL) += tcp.o
-OBJS-$(CONFIG_TLS_PROTOCOL) += tls.o
+OBJS-$(CONFIG_TLS_PROTOCOL) += tls_common.o
As James commented elsewhere, this could perhaps be kept as tls.c?
In general, this looks good to me.
The only thing I'm left wondering about is relating to this:
Also, this assumes lu_zeros patch to make GnuTLS and OpenSSL exclusive
has been applied.
These libraries are used for other things than implementing the TLS
protocol - the libraries' crypto stuff is ussed for implementing rtmpe://
as well. In that case only one of them is actually used, and they work as
replacements for each other, so it doesn't really hurt, but it's the
general concept I'm a bit weary about.
I'm trying to think how much work it would be to make this split version
not fail if more than one is enabled, to see if it'd be worth it:
One way, which OTOH brings back a bit more ifdefs, would be something
like this:
tls.c:
#ifdef CONFIG_GNUTLS
URLProtocol ff_tls_protocol = {
.name = "tls",
.url_open2 = tls_gnutls_open,
.url_read = tls_gnutls_read,
.url_write = tls_gnutls_write,
.url_close = tls_gnutls_close,
.priv_data_size = sizeof(TLSGnutlsContext),
.flags = URL_PROTOCOL_FLAG_NETWORK,
.priv_data_class = &tls_class,
};
#elif CONFIG_OPENSSL
...
This would require exposing the context definitions to tls.c though - not
pretty.
A separate alternative would be to name the protocol structs differently,
i.e. naming them ff_tls_gnutls_protocol, ff_tls_openssl_protocol, etc.
Then you'd have separate register-entries for them in allformats.c, and
you'd handle the mutual exclusion in configure like this:
tls_openssl_protocol_deps="!tls_gnutls_protocol" (see rtmp_protocol vs
librtmp_protocol).
I.e. it'd treat gnutls/openssl as generic external libraries that lavf can
use for various tasks, that sure can be enabled at the same time - the
only thing that you need one single copy of, is an actual implementation
of the tls:// protocol.
This would probably be a not too large change to your code, and would be
an even better separation of the implementation (where neither of them
fight over the same ff_tls_protocol symbol name).
What do you think - worthwhile or not?
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
