tests: make the CompareGuid() comparison size be less stupid

5f08e67 introduced a CompareGuid() call in the unit test harness, but unfortunately it has a typo and thus only ever compares the first pointer-sized word of the guid. With 4-GUIDs, this will usually produce the correct results; with 1-GUIDs it often won't. A second issue is that the memcmp() implementation of CompareGuid() produces a different sort order than comparing field-by-field, and also a different sort order than comparing the string representation. This is often not a problem (edk2, for example, never compares anything except equality of two GUIDs), but when writing test cases it is extremely helpful to be able to look at a list that is sorted in an intuitive order. This patch introduces a guidcmp() function in the test suite, which compares the binary data in the same order that comparing the two GUIDs' string representations would. Signed-off-by: Peter Jones <[email protected]>
jsjgruber · Sep 7, 2021 · 1f434aa · 1f434aa
1 parent 284f306
commit 1f434aa
Showing 1 changed file with 118 additions and 1 deletion.
diff --git a/include/test.h b/include/test.h
@@ -8,7 +8,13 @@
 #ifndef TEST_H_
 #define TEST_H_
 
+#define _GNU_SOURCE
+
 #include <stdarg.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <string.h>
+#include <inttypes.h>
 
 #if defined(__aarch64__)
 #include <aarch64/efibind.h>
@@ -61,7 +67,118 @@ extern EFI_SYSTEM_TABLE *ST;
 extern EFI_BOOT_SERVICES *BS;
 extern EFI_RUNTIME_SERVICES *RT;
 
-#define CompareGuid(a, b) memcmp(a, b, sizeof(a))
+#define GUID_FMT "%08x-%04hx-%04hx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
+#define GUID_ARGS(guid) \
+	((EFI_GUID)guid).Data1, ((EFI_GUID)guid).Data2, ((EFI_GUID)guid).Data3, \
+	((EFI_GUID)guid).Data4[1], ((EFI_GUID)guid).Data4[0], \
+	((EFI_GUID)guid).Data4[2], ((EFI_GUID)guid).Data4[3], \
+	((EFI_GUID)guid).Data4[4], ((EFI_GUID)guid).Data4[5], \
+	((EFI_GUID)guid).Data4[6], ((EFI_GUID)guid).Data4[7]
+
+static inline INT64
+guidcmp_helper(const EFI_GUID * const guid0, const EFI_GUID * const guid1)
+{
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+	printf("%s:%d:%s(): Comparing "GUID_FMT" to "GUID_FMT"\n",
+	       __FILE__, __LINE__-1, __func__,
+	       GUID_ARGS(*guid0), GUID_ARGS(*guid1));
+#endif
+
+	if (guid0->Data1 != guid1->Data1) {
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+		printf("%s:%d:%s(): returning 0x%"PRIx64"-0x%"PRIx64"->0x%"PRIx64"\n",
+		       __FILE__, __LINE__-1, __func__,
+		       (INT64)guid0->Data1, (INT64)guid1->Data1,
+		       (INT64)guid0->Data1 - (INT64)guid1->Data1);
+#endif
+		return (INT64)guid0->Data1 - (INT64)guid1->Data1;
+	}
+
+	if (guid0->Data2 != guid1->Data2) {
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+		printf("%s:%d:%s(): returning 0x%"PRIx64"-0x%"PRIx64"->0x%"PRIx64"\n",
+		       __FILE__, __LINE__-1, __func__,
+		       (INT64)guid0->Data2, (INT64)guid1->Data2,
+		       (INT64)guid0->Data2 - (INT64)guid1->Data2);
+#endif
+		return (INT64)guid0->Data2 - (INT64)guid1->Data2;
+	}
+
+	if (guid0->Data3 != guid1->Data3) {
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+		printf("%s:%d:%s(): returning 0x%"PRIx64"-0x%"PRIx64"->0x%"PRIx64"\n",
+		       __FILE__, __LINE__-1, __func__,
+		       (INT64)guid0->Data3, (INT64)guid1->Data3,
+		       (INT64)guid0->Data3 - (INT64)guid1->Data3);
+#endif
+		return (INT64)guid0->Data3 - (INT64)guid1->Data3;
+	}
+
+	/*
+	 * This is out of order because that's also true in the string
+	 * representation of it.
+	 */
+	if (guid0->Data4[1] != guid1->Data4[1]) {
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+		printf("%s:%d:%s(): returning 0x%"PRIx64"-0x%"PRIx64"->0x%"PRIx64"\n",
+		       __FILE__, __LINE__-1, __func__,
+		       (INT64)guid0->Data4[1], (INT64)guid1->Data4[1],
+		       (INT64)guid0->Data4[1] - (INT64)guid1->Data4[1]);
+#endif
+		return (INT64)guid0->Data4[1] - (INT64)guid1->Data4[1];
+	}
+
+	if (guid0->Data4[0] != guid1->Data4[0]) {
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+		printf("%s:%d:%s(): returning 0x%"PRIx64"-0x%"PRIx64"->0x%"PRIx64"\n",
+		       __FILE__, __LINE__-1, __func__,
+		       (INT64)guid0->Data4[0], (INT64)guid1->Data4[0],
+		       (INT64)guid0->Data4[0] - (INT64)guid1->Data4[0]);
+#endif
+		return (INT64)guid0->Data4[0] - (INT64)guid1->Data4[0];
+	}
+
+	for (UINTN i = 2; i < 8; i++) {
+		if (guid0->Data4[i] != guid1->Data4[i]) {
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+			printf("%s:%d:%s(): returning 0x%"PRIx64"-0x%"PRIx64"->0x%"PRIx64"\n",
+			       __FILE__, __LINE__-1, __func__,
+			       (INT64)guid0->Data4[i], (INT64)guid1->Data4[i],
+			       (INT64)guid0->Data4[i] - (INT64)guid1->Data4[i]);
+#endif
+			return (INT64)guid0->Data4[i] - (INT64)guid1->Data4[i];
+		}
+	}
+
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+	printf("%s:%d:%s(): returning 0x0\n",
+	       __FILE__, __LINE__-1, __func__);
+#endif
+	return 0;
+}
+
+static inline int
+guidcmp(const EFI_GUID * const guid0, const EFI_GUID * const guid1)
+{
+	INT64 cmp;
+	int ret;
+	EFI_GUID empty;
+	const EFI_GUID * const guida = guid0 ? guid0 : &empty;
+	const EFI_GUID * const guidb = guid1 ? guid1 : &empty;
+
+	SetMem(&empty, sizeof(EFI_GUID), 0);
+
+	cmp = guidcmp_helper(guida, guidb);
+	ret = cmp < 0 ? -1 : (cmp > 0 ? 1 : 0);
+#if (defined(SHIM_DEBUG) && SHIM_DEBUG != 0)
+	printf("%s:%d:%s():CompareGuid("GUID_FMT","GUID_FMT")->%lld (%d)\n",
+	       __FILE__, __LINE__-1, __func__,
+	       GUID_ARGS(*guida), GUID_ARGS(*guidb), cmp, ret);
+#endif
+	return ret;
+}
+
+#define CompareGuid(a, b) guidcmp(a, b)
 
 extern int debug;
 #ifdef dprint