Re: [Qemu-devel] [PATCH 01/10] crypto: introduce new module for computing hash digests

["Daniel P. Berrange" <berrange@redhat.com>]

On Thu, May 28, 2015 at 09:28:20PM +0800, Gonglei wrote:
> On 2015/5/21 18:56, Daniel P. Berrange wrote:
> > Introduce a new crypto/ directory that will (eventually) contain
> > all the cryptographic related code. This initially defines a
> > wrapper for initializing gnutls and for computing hashes with
> > gnutls. The former ensures that gnutls is guaranteed to be
> > initialized exactly once in QEMU regardless of CLI args. The
> > block quorum code currently fails to initialize gnutls so it
> > only works by luck, if VNC server TLS is not requested. The
> > hash APIs avoids the need to litter the rest of the code with
> > preprocessor checks and simplifies callers by allocating the
> > correct amount of memory for the requested hash.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

> > +##########################################
> > +# GNUTLS probe
> > +
> > +if test "$gnutls" != "no"; then
> > +    if $pkg_config --exists "gnutls"; then
> > +        gnutls_cflags=`$pkg_config --cflags gnutls`
> > +        gnutls_libs=`$pkg_config --libs gnutls`
> > +        libs_softmmu="$gnutls_libs $libs_softmmu"
> > +        libs_tools="$gnutls_libs $libs_tools"
> > +	QEMU_CFLAGS="$QEMU_CFLAGS $gnutls_cflags"
> > +        gnutls="yes"
> > +
> > +	# gnutls_hash_init requires >= 2.9.10
> 
> why 2.9.10 ? Isn't since 2.10.0 ?

I've double checked and 2.9.10 is correct according to the
gnutls NEWS file

[quote]
* Version 2.9.10 (released 2010-04-22)
...
** libgnutls: Exported API to access encryption and hash algorithms.
The new API functions are gnutls_cipher_decrypt, gnutls_cipher_deinit,
gnutls_cipher_encrypt, gnutls_cipher_get_block_size,
gnutls_cipher_init, gnutls_hash, gnutls_hash_deinit, gnutls_hash_fast,
gnutls_hash_get_len, gnutls_hash_init, gnutls_hash_output,
gnutls_hmac, gnutls_hmac_deinit, gnutls_hmac_fast,
gnutls_hmac_get_len, gnutls_hmac_init, gnutls_hmac_output.  New API
constants are GNUTLS_MAC_SHA224 and GNUTLS_DIG_SHA224.
[/quote]



> > diff --git a/vl.c b/vl.c
> > index 15bccc4..72313a4 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -119,6 +119,7 @@ int main(int argc, char **argv)
> >  #include "qapi/opts-visitor.h"
> >  #include "qom/object_interfaces.h"
> >  #include "qapi-event.h"
> > +#include "crypto/init.h"
> >  
> >  #define DEFAULT_RAM_SIZE 128
> >  
> > @@ -2777,6 +2778,7 @@ int main(int argc, char **argv, char **envp)
> >      uint64_t ram_slots = 0;
> >      FILE *vmstate_dump_file = NULL;
> >      Error *main_loop_err = NULL;
> > +    Error *err = NULL;
> >  
> >      qemu_init_cpu_loop();
> >      qemu_mutex_lock_iothread();
> > @@ -2819,6 +2821,12 @@ int main(int argc, char **argv, char **envp)
> >  
> >      runstate_init();
> >  
> > +    if (qcrypto_init(&err) < 0) {
> > +        fprintf(stderr, "Cannot initialize crypto: %s\n",
> > +                error_get_pretty(err));
> > +        error_free(err);
> 
> This free is superflous (before exit) IMO.

Personally I always free variables like this so valgrind doesn't complain
about leaks.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|