[PATCH for-4.6 0/2] Reduce the quantity of Coverity defects in testidl

[PATCH for-4.6 0/2] Reduce the quantity of Coverity defects in testidl

[Andrew Cooper]

No functional change, but removes 48 defects which get intermittently
reflagged every time the libxl IDL is altered.

Sent for 4.6 at Ian Campbells suggestion.

Andrew Cooper (2):
  tools/libxl: Assert success of memory allocation in testidl
  tools/libxl: Alter the use of rand() in testidl

 tools/libxl/gentest.py |   42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

-- 
1.7.10.4

[PATCH 1/2] tools/libxl: Assert success of memory allocation in testidl

[Andrew Cooper]

The chances of an allocation failing are slim but nonzero.  Assert
success of each allocation to quieten Coverity, which re-notices defects
each time the IDL changes.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentest.py |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 7621a1e..85311e7 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -33,6 +33,7 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
         s += "%s = rand()%%8;\n" % (parent + ty.lenvar.name)
         s += "%s = calloc(%s, sizeof(*%s));\n" % \
             (v, parent + ty.lenvar.name, v)
+        s += "assert(%s);\n" % (v, )
         s += "{\n"
         s += "    int i;\n"
         s += "    for (i=0; i<%s; i++)\n" % (parent + ty.lenvar.name)
@@ -98,6 +99,7 @@ if __name__ == '__main__':
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 
 #include "libxl.h"
 #include "libxl_utils.h"
@@ -106,6 +108,7 @@ static char *rand_str(void)
 {
     int i, sz = rand() % 32;
     char *s = malloc(sz+1);
+    assert(s);
     for (i=0; i<sz; i++)
         s[i] = 'a' + (rand() % 26);
     s[i] = '\\0';
@@ -124,6 +127,7 @@ static void libxl_bitmap_rand_init(libxl_bitmap *bitmap)
     int i;
     bitmap->size = rand() % 16;
     bitmap->map = calloc(bitmap->size, sizeof(*bitmap->map));
+    assert(bitmap->map);
     libxl_for_each_bit(i, *bitmap) {
         if (rand() % 2)
             libxl_bitmap_set(bitmap, i);
@@ -136,6 +140,7 @@ static void libxl_key_value_list_rand_init(libxl_key_value_list *pkvl)
 {
     int i, nr_kvp = rand() % 16;
     libxl_key_value_list kvl = calloc(nr_kvp+1, 2*sizeof(char *));
+    assert(kvl);
 
     for (i = 0; i<2*nr_kvp; i += 2) {
         kvl[i] = rand_str();
@@ -196,6 +201,7 @@ static void libxl_string_list_rand_init(libxl_string_list *p)
 {
     int i, nr = rand() % 16;
     libxl_string_list l = calloc(nr+1, sizeof(char *));
+    assert(l);
 
     for (i = 0; i<nr; i++) {
         l[i] = rand_str();
-- 
1.7.10.4

[PATCH 2/2] tools/libxl: Alter the use of rand() in testidl

[Andrew Cooper]

Coverity warns for every occurrence of rand(), which is made worse
because each time the IDL changes, some of the calls get re-flagged.

Collect all calls to rand() in a single function, test_rand(), which
takes a modulo parameter for convenience.  This turns 40 defects
currently into 1, which won't get re-flagged when the IDL changes.

In addition, fix the erroneous random choice for libxl_defbool_set().
"!!rand() % 1" is unconditionally 0, and even without the "% 1" would
still be very heavily skewed in one direction.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentest.py |   36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 85311e7..989959f 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -30,7 +30,7 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
     elif isinstance(ty, idl.Array):
         if parent is None:
             raise Exception("Array type must have a parent")
-        s += "%s = rand()%%8;\n" % (parent + ty.lenvar.name)
+        s += "%s = test_rand(8);\n" % (parent + ty.lenvar.name)
         s += "%s = calloc(%s, sizeof(*%s));\n" % \
             (v, parent + ty.lenvar.name, v)
         s += "assert(%s);\n" % (v, )
@@ -64,13 +64,13 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
     elif ty.typename in ["libxl_uuid", "libxl_mac", "libxl_hwcap", "libxl_ms_vm_genid"]:
         s += "rand_bytes((uint8_t *)%s, sizeof(*%s));\n" % (v,v)
     elif ty.typename in ["libxl_domid", "libxl_devid"] or isinstance(ty, idl.Number):
-        s += "%s = rand() %% (sizeof(%s)*8);\n" % \
+        s += "%s = test_rand(sizeof(%s) * 8);\n" % \
              (ty.pass_arg(v, parent is None),
               ty.pass_arg(v, parent is None))
     elif ty.typename in ["bool"]:
-        s += "%s = rand() %% 2;\n" % v
+        s += "%s = test_rand(2);\n" % v
     elif ty.typename in ["libxl_defbool"]:
-        s += "libxl_defbool_set(%s, !!rand() %% 1);\n" % v
+        s += "libxl_defbool_set(%s, test_rand(2));\n" % v
     elif ty.typename in ["char *"]:
         s += "%s = rand_str();\n" % v
     elif ty.private:
@@ -104,13 +104,19 @@ if __name__ == '__main__':
 #include "libxl.h"
 #include "libxl_utils.h"
 
+static int test_rand(unsigned max)
+{
+    /* We are not using rand() for its cryptographic properies. */
+    return rand() % max;
+}
+
 static char *rand_str(void)
 {
-    int i, sz = rand() % 32;
+    int i, sz = test_rand(32);
     char *s = malloc(sz+1);
     assert(s);
     for (i=0; i<sz; i++)
-        s[i] = 'a' + (rand() % 26);
+        s[i] = 'a' + test_rand(26);
     s[i] = '\\0';
     return s;
 }
@@ -119,17 +125,17 @@ static void rand_bytes(uint8_t *p, size_t sz)
 {
     int i;
     for (i=0; i<sz; i++)
-        p[i] = rand() % 256;
+        p[i] = test_rand(256);
 }
 
 static void libxl_bitmap_rand_init(libxl_bitmap *bitmap)
 {
     int i;
-    bitmap->size = rand() % 16;
+    bitmap->size = test_rand(16);
     bitmap->map = calloc(bitmap->size, sizeof(*bitmap->map));
     assert(bitmap->map);
     libxl_for_each_bit(i, *bitmap) {
-        if (rand() % 2)
+        if (test_rand(2))
             libxl_bitmap_set(bitmap, i);
         else
             libxl_bitmap_reset(bitmap, i);
@@ -138,13 +144,13 @@ static void libxl_bitmap_rand_init(libxl_bitmap *bitmap)
 
 static void libxl_key_value_list_rand_init(libxl_key_value_list *pkvl)
 {
-    int i, nr_kvp = rand() % 16;
+    int i, nr_kvp = test_rand(16);
     libxl_key_value_list kvl = calloc(nr_kvp+1, 2*sizeof(char *));
     assert(kvl);
 
     for (i = 0; i<2*nr_kvp; i += 2) {
         kvl[i] = rand_str();
-        if (rand() % 8)
+        if (test_rand(8))
             kvl[i+1] = rand_str();
         else
             kvl[i+1] = NULL;
@@ -156,7 +162,7 @@ static void libxl_key_value_list_rand_init(libxl_key_value_list *pkvl)
 
 static void libxl_cpuid_policy_list_rand_init(libxl_cpuid_policy_list *pp)
 {
-    int i, nr_policies = rand() % 16;
+    int i, nr_policies = test_rand(16);
     struct {
         const char *n;
         int w;
@@ -189,8 +195,8 @@ static void libxl_cpuid_policy_list_rand_init(libxl_cpuid_policy_list *pp)
     libxl_cpuid_policy_list p = NULL;
 
     for (i = 0; i < nr_policies; i++) {
-        int opt = rand() % nr_options;
-        int val = rand() % (1<<options[opt].w);
+        int opt = test_rand(nr_options);
+        int val = test_rand(1<<options[opt].w);
         snprintf(buf, 64, \"%s=%#x\", options[opt].n, val);
         libxl_cpuid_parse_config(&p, buf);
     }
@@ -199,7 +205,7 @@ static void libxl_cpuid_policy_list_rand_init(libxl_cpuid_policy_list *pp)
 
 static void libxl_string_list_rand_init(libxl_string_list *p)
 {
-    int i, nr = rand() % 16;
+    int i, nr = test_rand(16);
     libxl_string_list l = calloc(nr+1, sizeof(char *));
     assert(l);
 
-- 
1.7.10.4

Re: [PATCH 1/2] tools/libxl: Assert success of memory allocation in testidl

[Wei Liu]

On Fri, Aug 07, 2015 at 03:06:23PM +0100, Andrew Cooper wrote:
> The chances of an allocation failing are slim but nonzero.  Assert
> success of each allocation to quieten Coverity, which re-notices defects
> each time the IDL changes.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH 2/2] tools/libxl: Alter the use of rand() in testidl

[Wei Liu]

On Fri, Aug 07, 2015 at 03:06:24PM +0100, Andrew Cooper wrote:
> Coverity warns for every occurrence of rand(), which is made worse
> because each time the IDL changes, some of the calls get re-flagged.
> 
> Collect all calls to rand() in a single function, test_rand(), which
> takes a modulo parameter for convenience.  This turns 40 defects
> currently into 1, which won't get re-flagged when the IDL changes.
> 
> In addition, fix the erroneous random choice for libxl_defbool_set().
> "!!rand() % 1" is unconditionally 0, and even without the "% 1" would
> still be very heavily skewed in one direction.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH for-4.6 0/2] Reduce the quantity of Coverity defects in testidl

[Wei Liu]

On Fri, Aug 07, 2015 at 03:06:22PM +0100, Andrew Cooper wrote:
> No functional change, but removes 48 defects which get intermittently
> reflagged every time the libxl IDL is altered.
> 
> Sent for 4.6 at Ian Campbells suggestion.
> 
> Andrew Cooper (2):
>   tools/libxl: Assert success of memory allocation in testidl
>   tools/libxl: Alter the use of rand() in testidl
> 
>  tools/libxl/gentest.py |   42 +++++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 15 deletions(-)
> 

This series only affects idl test case which is not possible to cause
any regression. Feel free to apply it when convenient.

Wei.

> -- 
> 1.7.10.4