From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35379) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKSKi-00080p-Q4 for qemu-devel@nongnu.org; Tue, 05 Jul 2016 11:32:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKSKh-0001gp-IM for qemu-devel@nongnu.org; Tue, 05 Jul 2016 11:32:20 -0400 Date: Tue, 5 Jul 2016 16:32:07 +0100 From: "Daniel P. Berrange" Message-ID: <20160705153207.GZ6553@redhat.com> Reply-To: "Daniel P. Berrange" References: <1467715800-20379-1-git-send-email-berrange@redhat.com> <1467715800-20379-2-git-send-email-berrange@redhat.com> <577BCC3E.2000807@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <577BCC3E.2000807@redhat.com> Subject: Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Kevin Wolf , Fam Zheng , qemu-block@nongnu.org, Max Reitz , Alberto Garcia On Tue, Jul 05, 2016 at 09:03:26AM -0600, Eric Blake wrote: > On 07/05/2016 04:49 AM, Daniel P. Berrange wrote: > > GLib >= 2.16 provides GChecksum API which is good enough > > for md5, sha1, sha256 and sha512. Use this as a final > > fallback if neither nettle or gcrypt are available. This > > lets us remove the stub hash impl, and so callers can > > be sure those 4 algs are always available at compile > > time. They may still be disabled at runtime, so a check > > for qcrypto_hash_supports() is still best practice to > > report good error messages. > > > > Signed-off-by: Daniel P. Berrange > > --- > > crypto/Makefile.objs | 2 +- > > crypto/hash-glib.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > crypto/hash-stub.c | 41 ----------------------- > > 3 files changed, 95 insertions(+), 42 deletions(-) > > create mode 100644 crypto/hash-glib.c > > delete mode 100644 crypto/hash-stub.c > > > > > +gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg) > > +{ > > + if (alg < G_N_ELEMENTS(qcrypto_hash_alg_map) && > > + qcrypto_hash_alg_map[alg] != -1) { > > + return true; > > + } > > + return false; > > +} > > + > > + > > +int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg, > > + const struct iovec *iov, > > + size_t niov, > > + uint8_t **result, > > + size_t *resultlen, > > + Error **errp) > > +{ > > + int i, ret; > > + GChecksum *cs; > > + > > + if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_map) || > > + qcrypto_hash_alg_map[alg] == -1) { > > Worth writing this as 'if (!gcrypto_hash_supports(alg)) {' ? This pattern is used in the nettle + gcrypt impls too. I'd be happy to switch to what you suggest in all impls separately. > Otherwise, > Reviewed-by: Eric Blake Oh, and BTW the pre-existing test-crypto-hash unit tests will already provide coverage for this implementation & i've checked it passes when --disable-nettle --disable-gcrypt are given to confnigure. 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 :|