Re: [libvirt] [PATCH 1/3] tests: Add mock for virRandomBytes
On 05/19/2016 03:32 AM, Peter Krempa wrote:
On Wed, May 18, 2016 at 19:52:30 -0400, John Ferlan wrote:
> Create a mock for virRandomBytes to generate a not so random value that
> can be used by the tests to ensure the generation of an encrypted secret
> by masterKey and random initialization vector can produce an expected
> result. The "random number" generated is based upon the size of the
> expected stream of bytes being returned where each byte in the result
> gets the index of the array - hence a 4 byte array returns 0x00010203.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> tests/qemuxml2argvmock.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index 1616eed..dade748 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
[...]
> @@ -145,3 +152,25 @@ virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
> {
> /* nada */
> }
> +
> +int
> +virRandomBytes(unsigned char *buf,
> + size_t buflen)
> +{
> + size_t i;
> +
> + for (i = 0; i < buflen; i++)
> + buf[i] = i;
> +
> + return 0;
> +}
> +
> +#ifdef WITH_GNUTLS
> +int
> +gnutls_rnd(gnutls_rnd_level_t level ATTRIBUTE_UNUSED,
> + void *data,
> + size_t len)
> +{
> + return virRandomBytes(data, len);
> +#endif
As I've pointed out last time, this won't compile without gnutls.
Beyond the merge issue with putting the } after the #endif, I agree - it
won't compile, but I can only assume that's not your issue; otherwise, I
would think that your initial review would have just pointed out that
the } needs to be inside the #endif.
If one checks who would actually call this:
qemuDomainGenerateRandomKey():
#if HAVE_GNUTLS_RND
/* Generate a master key using gnutls_rnd() if possible */
if ((ret = gnutls_rnd(GNUTLS_RND_RANDOM, key, nbytes)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
...
So is your issue that
1. The incorrect placement of #endif
2. "#ifdef HAVE_GNUTLS_RND" should have been used
3. you want an "#else"
4. you don't want to see the #ifdef?
I could see value in #2 to follow the caller, but the others I don't see
value in. But certainly none of those options encompasses the catch-all
this won't compile without gnutls when I read the review.
In any case, between patch 2 and the cover - I pointed out that I wasn't
sure putting this into a file named "qemuxml2argvmock.c" was the right
choice given that as you pointed out previously that qemu isn't required.
So I started down the path of creating a virrandommock.c and a
virrandomtest.c; however, there is just something about that mock
environment that I don't understand well enough to get things to work as
I expected. I posted this with the hope that someone would be able to
look and provide some assistance with the magic words to write in
Makefile.am.
OH and BTW: In patch 2, the }/#endif issue was already handled properly
in virrandommock.c.
John