Add unit tests for utility.c

Also fixes returned value from Memset().  And SafeMemcmp() should
return 0 (equal) if comparing 0 bytes, to match the behavior of memcmp().

BUG=chromium-os:17564
TEST=make && make runtests

Change-Id: Id43e70eecf04815216e1fd952271af35e0a66396
Reviewed-on: http://gerrit.chromium.org/gerrit/6539
Reviewed-by: Stefan Reinauer <reinauer@google.com>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Tested-by: Randall Spangler <rspangler@chromium.org>

4 files changed, 80 insertions, 8 deletions

diff --git a/firmware/include/utility.h b/firmware/include/utility.h
index 7efbd491..8676345d 100644
--- a/firmware/include/utility.h
+++ b/firmware/include/utility.h
@@ -64,12 +64,16 @@ void* Memcpy(void* dest, const void* src, uint64_t n);
 /* Implementations of the functions below must be built as part of the firmware
  * and defined in lib/utility.c */
 
-/* Set [n] bytes starting at [s] to [c]. */
-void* Memset(void *dest, const uint8_t c, uint64_t n);
+/* Set [n] bytes starting at [s] to [c].  Returns dest. */
+void* Memset(void* dest, const uint8_t c, uint64_t n);
 
-/* Compare [n] bytes starting at [s1] with [s2] and return 0 if they match,
- * 1 if they don't. Time taken to perform the comparison is only dependent on
- * [n] and not on the relationship of the match between [s1] and [s2].
+/* Compare [n] bytes starting at [s1] with [s2] and return 0 if they
+ * match, 1 if they don't.  Returns 0 if n=0, since no bytes mismatched.
+ * Time taken to perform the comparison is only dependent on [n] and
+ * not on the relationship of the match between [s1] and [s2].
+ *
+ * Note that unlike Memcmp(), this only indicates inequality, not
+ * whether s1 is less than or greater than s2.
  */
 int SafeMemcmp(const void* s1, const void* s2, size_t n);
 
diff --git a/firmware/lib/utility.c b/firmware/lib/utility.c
index 3f3ed5d5..7a2370b0 100644
--- a/firmware/lib/utility.c
+++ b/firmware/lib/utility.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+/* Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  *
@@ -13,7 +13,7 @@ void* Memset(void* d, const uint8_t c, uint64_t n) {
   while (n--) {
     *dest++ = c;
   }
-  return dest;
+  return d;
 }
 
 int SafeMemcmp(const void* s1, const void* s2, size_t n) {
@@ -22,7 +22,7 @@ int SafeMemcmp(const void* s1, const void* s2, size_t n) {
   int result = 0;
 
   if (0 == n)
-    return 1;
+    return 0;
 
   /* Code snippet without data-dependent branch due to
    * Nate Lawson (nate@root.org) of Root Labs. */
diff --git a/tests/Makefile b/tests/Makefile
index e2a4af53..01729598 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -16,6 +16,7 @@ TEST_NAMES = cgptlib_test \
 	     sha_benchmark \
 	     sha_tests \
 	     utility_string_tests \
+	     utility_tests \
 	     vboot_common_tests \
 	     vboot_common2_tests \
 	     vboot_common3_tests \
@@ -92,6 +93,7 @@ runcryptotests:
 runmisctests:
 	./run_vbutil_tests.sh
 	${BUILD_ROOT}/utility_string_tests
+	${BUILD_ROOT}/utility_tests
 
 #This will exercise vbutil_kernel and vbutil_firmware
 runfuzztests:
diff --git a/tests/utility_tests.c b/tests/utility_tests.c
new file mode 100644
index 00000000..b8a1da93
--- /dev/null
+++ b/tests/utility_tests.c
@@ -0,0 +1,66 @@
+/* Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Tests for utility functions.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define _STUB_IMPLEMENTATION_  /* So we can use memset() ourselves */
+
+#include "test_common.h"
+#include "utility.h"
+#include "vboot_common.h"
+
+
+/* Test Memset */
+static void MemsetTest(void) {
+  char dest[128];
+  char want[128];
+
+  memset(want, 0, 128);
+  memset(dest, 0, 128);
+
+  /* Simple fill */
+  memset(want, 123, 5);
+  TEST_EQ(0, dest - (char*)Memset(dest, 123, 5), "Memset() returns dest");
+  TEST_EQ(0, memcmp(dest, want, 128), "Memset()");
+
+  /* Filling length 0 does nothing */
+  Memset(dest, 42, 0);
+  TEST_EQ(0, memcmp(dest, want, 128), "Memset() size=0");
+}
+
+
+/* Test SafeMemcmp */
+static void SafeMemcmpTest(void) {
+  /* Zero-length strings are equal */
+  TEST_EQ(0, SafeMemcmp("APPLE", "TIGER", 0), "SafeMemcmp() size=0");
+
+  /* Test equal arrays */
+  TEST_EQ(0, SafeMemcmp("clonebob", "clonebob", 8), "SafeMemcmp() equal");
+  /* Inequality past end of array doesn't affect result */
+  TEST_EQ(0, SafeMemcmp("clonebob", "clonedan", 5), "SafeMemcmp() equal2");
+
+  TEST_EQ(1, SafeMemcmp("APPLE", "TIGER", 5), "SafeMemcmp() unequal");
+  TEST_EQ(1, SafeMemcmp("APPLE", "APPLe", 5), "SafeMemcmp() unequal 2");
+}
+
+
+/* disable MSVC warnings on unused arguments */
+__pragma(warning (disable: 4100))
+
+int main(int argc, char* argv[]) {
+  int error_code = 0;
+
+  MemsetTest();
+  SafeMemcmpTest();
+
+  if (!gTestSuccess)
+    error_code = 255;
+
+  return error_code;
+}