On Fri, 22 May 2015, Martin Storsjö wrote:
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?
Just to clarify - if others feel that this is way too much work/mess for
an academical question, I don't oppose going for it this way with Luca's
patch, but I wanted to raise the question first at least.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
