[flac-dev] [PATCH] Optionally, allow distros to use openssl for MD5 verification

2012-05-05 Thread Cristian Rodríguez 
This has the advantage of being more efficient than the included
routines and allows distros to centralize crypto mainteniance on
a few libraries.
---
 configure.ac  |4 +-
 m4/ax_check_openssl.m4|  124 +
 src/libFLAC/Makefile.am   |2 +-
 src/libFLAC/include/private/md5.h |8 ++-
 src/libFLAC/md5.c |   38 +++
 src/libFLAC/stream_decoder.c  |   30 +++--
 src/libFLAC/stream_encoder.c  |   30 +++--
 7 files changed, 220 insertions(+), 16 deletions(-)
 create mode 100644 m4/ax_check_openssl.m4

diff --git a/configure.ac b/configure.ac
index e794ca2..3b627e5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -56,7 +56,7 @@ AM_PROG_CC_C_O
 AC_C_INLINE
 AC_C_VARARRAYS
 AC_C_TYPEOF
-
+AC_FUNC_ALLOCA
 AC_CHECK_HEADERS(stdint.h)
 AC_SUBST(HAVE_STDINT_H)
 AC_CHECK_HEADERS(inttypes.h)
@@ -83,6 +83,8 @@ dnl check for getopt in standard library
 dnl AC_CHECK_FUNCS(getopt_long , , [LIBOBJS="$LIBOBJS getopt.o getopt1.o"] )
 AC_CHECK_FUNCS(getopt_long, [], [])
 
+AX_CHECK_OPENSSL([AC_DEFINE([HAVE_OPENSSL], [1], [We have openSSL])])
+
 case "$host_cpu" in
i*86)
cpu_ia32=true
diff --git a/m4/ax_check_openssl.m4 b/m4/ax_check_openssl.m4
new file mode 100644
index 000..a87c5a6
--- /dev/null
+++ b/m4/ax_check_openssl.m4
@@ -0,0 +1,124 @@
+# ===
+# http://www.gnu.org/software/autoconf-archive/ax_check_openssl.html
+# ===
+#
+# SYNOPSIS
+#
+#   AX_CHECK_OPENSSL([action-if-found[, action-if-not-found]])
+#
+# DESCRIPTION
+#
+#   Look for OpenSSL in a number of default spots, or in a user-selected
+#   spot (via --with-openssl).  Sets
+#
+# OPENSSL_INCLUDES to the include directives required
+# OPENSSL_LIBS to the -l directives required
+# OPENSSL_LDFLAGS to the -L or -R flags required
+#
+#   and calls ACTION-IF-FOUND or ACTION-IF-NOT-FOUND appropriately
+#
+#   This macro sets OPENSSL_INCLUDES such that source files should use the
+#   openssl/ directory in include directives:
+#
+# #include 
+#
+# LICENSE
+#
+#   Copyright (c) 2009,2010 Zmanda Inc. 
+#   Copyright (c) 2009,2010 Dustin J. Mitchell 
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 8
+
+AU_ALIAS([CHECK_SSL], [AX_CHECK_OPENSSL])
+AC_DEFUN([AX_CHECK_OPENSSL], [
+found=false
+AC_ARG_WITH([openssl],
+[AS_HELP_STRING([--with-openssl=DIR],
+[root of the OpenSSL directory])],
+[
+case "$withval" in
+"" | y | ye | yes | n | no)
+AC_MSG_ERROR([Invalid --with-openssl value])
+  ;;
+*) ssldirs="$withval"
+  ;;
+esac
+], [
+# if pkg-config is installed and openssl has installed a .pc file,
+# then use that information and don't search ssldirs
+AC_PATH_PROG([PKG_CONFIG], [pkg-config])
+if test x"$PKG_CONFIG" != x""; then
+OPENSSL_LDFLAGS=`$PKG_CONFIG openssl --libs-only-L 2>/dev/null`
+if test $? = 0; then
+OPENSSL_LIBS=`$PKG_CONFIG openssl --libs-only-l 
2>/dev/null`
+OPENSSL_INCLUDES=`$PKG_CONFIG openssl --cflags-only-I 
2>/dev/null`
+found=true
+fi
+fi
+
+# no such luck; use some default ssldirs
+if ! $found; then
+ssldirs="/usr/local/ssl /usr/lib/ssl /usr/ssl /usr/pkg 
/usr/local /usr"
+fi
+]
+)
+
+
+# note that we #include , so the OpenSSL headers have to be 
in
+# an 'openssl' subdirectory
+
+if ! $found; then
+OPENSSL_INCLUDES=
+for ssldir in $ssldirs; do
+AC_MSG_CHECKING([for openssl/ssl.h in $ssldir])
+if test -f "$ssldir/include/openssl/ssl.h"; then
+OPENSSL_INCLUDES="-I$ssldir/include"
+OPENSSL_LDFLAGS="-L$ssldir/lib"
+OPENSSL_LIBS="-lssl -lcrypto"
+found=true
+AC_MSG_RESULT([yes])
+break
+else
+AC_MSG_RESULT([no])
+fi
+done
+
+# if the file wasn't found, well, go ahead and try the link anyway -- 
maybe
+# it will just work!
+fi
+
+# try the preprocessor and linker with our new flags,
+# being careful not to pollute the global LIBS, LDFLAGS, and CPPFLAGS
+
+AC_MSG_CHECKING([whether compiling and linking against OpenSSL works])
+echo "Trying link with OPENSSL_LDFLAGS=$OPENSSL_LDFLAGS;" \
+"OPENSSL_LIBS=$OPENSSL_LIBS; OPENSSL_INCLUDES=$OPE

Re: [flac-dev] [PATCH] Optionally, allow distros to use openssl for MD5 verification

2012-05-05 Thread Eric Wong 
Cristian Rodríguez  wrote:
> +#if defined(HAVE_OPENSSL)
> +/* decoder->private_->computed_md5sum is NULL when 
> decoder->private_->do_md5_checking == false
> +* that causes assertion failure crash in openSSL.
> +*/
> +if(decoder->private_->do_md5_checking) {
> +md5_failed = (EVP_DigestFinal_ex(&decoder->private_->md5context, 
> decoder->private_->computed_md5sum, NULL) == 0);
> +}
> +#else
>   FLAC__MD5Final(decoder->private_->computed_md5sum, 
> &decoder->private_->md5context);
> -
> +#endif

Can you do this without sprinkling #ifdefs all over the place?

Mixing #ifdefs and normal C control structures make code hard to
read/maintain.  This is *especially* true for folks who aren't regular
contributors to flac, myself included.

I would define workalike macros/no-op functions instead and hide
OpenSSL-related functionality behind them.

Thanks.
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] [PATCH] Optionally, allow distros to use openssl for MD5 verification

2012-05-05 Thread Erik de Castro Lopo 
Eric Wong wrote:

> Can you do this without sprinkling #ifdefs all over the place?

+1

> Mixing #ifdefs and normal C control structures make code hard to
> read/maintain.  This is *especially* true for folks who aren't regular
> contributors to flac, myself included.

Very much agree.

Erik
-- 
--
Erik de Castro Lopo
http://www.mega-nerd.com/
___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev

Re: [flac-dev] [PATCH] Optionally, allow distros to use openssl for MD5 verification

2012-05-05 Thread Cristian Rodríguez 
El 05/05/12 21:04, Eric Wong escribió:
> Cristian Rodríguez  wrote:
>> +#if defined(HAVE_OPENSSL)
>> +/* decoder->private_->computed_md5sum is NULL when 
>> decoder->private_->do_md5_checking == false
>> +* that causes assertion failure crash in openSSL.
>> +*/
>> +if(decoder->private_->do_md5_checking) {
>> +md5_failed = (EVP_DigestFinal_ex(&decoder->private_->md5context, 
>> decoder->private_->computed_md5sum, NULL) == 0);
>> +}
>> +#else
>>  
>> FLAC__MD5Final(decoder->private_->computed_md5sum,&decoder->private_->md5context);
>> -
>> +#endif
>
> Can you do this without sprinkling #ifdefs all over the place?
>
> Mixing #ifdefs and normal C control structures make code hard to
> read/maintain.  This is *especially* true for folks who aren't regular
> contributors to flac, myself included.
>
> I would define workalike macros/no-op functions instead and hide
> OpenSSL-related functionality behind them.

Well, there are reasons why I did it this way.. Initially I though about 
your suggested approach...

With this new code

- The format can be easily extended to support other digests algorithms, 
(i.e shaXXX that nowdays is implemented with hardware support) openssl 
is everywhere including windows.

- The original code follows the traditional old-style convention of 
init() update() final() for every operation, this new version init() 
cleanup() only *once* per encoder/decoder "instance" reducing the number 
of function calls and openssl reuses the allocated resources...

___
flac-dev mailing list
flac-dev@xiph.org
http://lists.xiph.org/mailman/listinfo/flac-dev