[libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form. Signed-off-by: Ján Tomko --- src/util/vircrypto.c | 31 +-- src/util/vircrypto.h | 7 +++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); int -virCryptoHashString(virCryptoHash hash, -const char *input, -char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; -size_t hashstrlen; -size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } -hashstrlen = (hashinfo[hash].hashlen * 2) + 1; - -if (!(hashinfo[hash].func(input, strlen(input), buf))) { +if (!(hashinfo[hash].func(input, strlen(input), output))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to compute hash of data")); return -1; } +return 0; +} + +int +virCryptoHashString(virCryptoHash hash, +const char *input, +char **output) +{ +unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; +size_t hashstrlen; +size_t i; + +if (virCryptoHashBuf(hash, input, buf) < 0) +return -1; + +hashstrlen = (hashinfo[hash].hashlen * 2) + 1; + if (VIR_ALLOC_N(*output, hashstrlen) < 0) return -1; diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h index 81743d2f74..64984006be 100644 --- a/src/util/vircrypto.h +++ b/src/util/vircrypto.h @@ -41,6 +41,13 @@ typedef enum { VIR_CRYPTO_CIPHER_LAST } virCryptoCipher; +int +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) +ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +ATTRIBUTE_RETURN_CHECK; + int virCryptoHashString(virCryptoHash hash, const char *input, -- 2.16.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
On 05/11/2018 05:50 PM, Ján Tomko wrote: > A function that keeps the hash in binary form instead of converting > it to human-readable hexadecimal form. > > Signed-off-by: Ján Tomko > --- > src/util/vircrypto.c | 31 +-- > src/util/vircrypto.h | 7 +++ > 2 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c > index 48b04fc8ce..1a2dcc28b7 100644 > --- a/src/util/vircrypto.c > +++ b/src/util/vircrypto.c > @@ -54,28 +54,39 @@ struct virHashInfo { > verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); > > int > -virCryptoHashString(virCryptoHash hash, > -const char *input, > -char **output) > +virCryptoHashBuf(virCryptoHash hash, > + const char *input, > + unsigned char *output) > { > -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; > -size_t hashstrlen; > -size_t i; > - > if (hash >= VIR_CRYPTO_HASH_LAST) { > virReportError(VIR_ERR_INVALID_ARG, > _("Unknown crypto hash %d"), hash); > return -1; > } This check feels useless. It's like if we were checking a value before passing it to vir*TypeToString(). But it's pre-existing, so you have my ACK. But a follow up patch removing it (=trivial) would be nice. Also, don't forget to export the symbol in libvirt_private.syms ;-) I'm wondering how linker succeeds in 3/5 where you use the function without it being exported. Maybe my compiler decided to inline this function? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
On Mon, May 14, 2018 at 11:17:51AM +0200, Michal Privoznik wrote: On 05/11/2018 05:50 PM, Ján Tomko wrote: A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form. Signed-off-by: Ján Tomko --- src/util/vircrypto.c | 31 +-- src/util/vircrypto.h | 7 +++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); int -virCryptoHashString(virCryptoHash hash, -const char *input, -char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; -size_t hashstrlen; -size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } This check feels useless. It's like if we were checking a value before passing it to vir*TypeToString(). But it's pre-existing, so you have my ACK. But a follow up patch removing it (=trivial) would be nice. On one hand, this check should be pointless if the rest of our code is correct. On the other hand, our functions in src/util usually do perform some validation of the parameters. Although it could be transformed to use virReportEnumRangeError. Also, don't forget to export the symbol in libvirt_private.syms ;-) I'm wondering how linker succeeds in 3/5 where you use the function without it being exported. Maybe my compiler decided to inline this function? Thanks, fixed. Jano Michal signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
On 05/11/2018 11:50 AM, Ján Tomko wrote: A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form. Signed-off-by: Ján Tomko --- src/util/vircrypto.c | 31 +-- src/util/vircrypto.h | 7 +++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); int -virCryptoHashString(virCryptoHash hash, -const char *input, -char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; -size_t hashstrlen; -size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } -hashstrlen = (hashinfo[hash].hashlen * 2) + 1; - -if (!(hashinfo[hash].func(input, strlen(input), buf))) { +if (!(hashinfo[hash].func(input, strlen(input), output))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to compute hash of data")); return -1; } +return 0; +} + +int +virCryptoHashString(virCryptoHash hash, +const char *input, +char **output) +{ +unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; +size_t hashstrlen; +size_t i; + +if (virCryptoHashBuf(hash, input, buf) < 0) +return -1; + +hashstrlen = (hashinfo[hash].hashlen * 2) + 1; + How about virCryptoHashBuf returning the number of raw bytes it produced so that not all callers need to look it up? Stefan if (VIR_ALLOC_N(*output, hashstrlen) < 0) return -1; diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h index 81743d2f74..64984006be 100644 --- a/src/util/vircrypto.h +++ b/src/util/vircrypto.h @@ -41,6 +41,13 @@ typedef enum { VIR_CRYPTO_CIPHER_LAST } virCryptoCipher; +int +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) +ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +ATTRIBUTE_RETURN_CHECK; + int virCryptoHashString(virCryptoHash hash, const char *input, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list