[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]