vboot: stop using StrnAppend and Uint64ToString

Use snprintf instead.  Remove utility_string library.
Also, prepare VbDisplayDebugInfo to handle 64-byte nvdata.

BUG=b:124141368, chromium:968464
TEST=make clean && make runtests
TEST=boot with 16-byte nvdata, check <TAB> output (one line)
TEST=boot with 64-byte nvdata, check <TAB> output (five lines)
BRANCH=none

Change-Id: If6c4b3a4e9fa7b71cb2d8ca7ccd37e4f36b97fd6
Signed-off-by: Joel Kitching <kitching@google.com>
Cq-Depend: chromium:2122061
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2104880
Tested-by: Joel Kitching <kitching@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

15 files changed, 35 insertions, 191 deletions

diff --git a/firmware/lib/cgptlib/cgptlib.c b/firmware/lib/cgptlib/cgptlib.c
index db704b54..2cc77b4a 100644
--- a/firmware/lib/cgptlib/cgptlib.c
+++ b/firmware/lib/cgptlib/cgptlib.c
@@ -9,7 +9,6 @@
 #include "cgptlib_internal.h"
 #include "crc32.h"
 #include "gpt.h"
-#include "utility.h"
 #include "vboot_api.h"
 
 int GptInit(GptData *gpt)
diff --git a/firmware/lib/cgptlib/cgptlib_internal.c b/firmware/lib/cgptlib/cgptlib_internal.c
index 7d214a91..593b1e2c 100644
--- a/firmware/lib/cgptlib/cgptlib_internal.c
+++ b/firmware/lib/cgptlib/cgptlib_internal.c
@@ -9,7 +9,6 @@
 #include "crc32.h"
 #include "gpt.h"
 #include "gpt_misc.h"
-#include "utility.h"
 
 const static int MIN_SECTOR_SIZE = 512;
 
diff --git a/firmware/lib/gpt_misc.c b/firmware/lib/gpt_misc.c
index 6d91ec15..ae7f8a23 100644
--- a/firmware/lib/gpt_misc.c
+++ b/firmware/lib/gpt_misc.c
@@ -9,7 +9,6 @@
 #include "cgptlib_internal.h"
 #include "crc32.h"
 #include "gpt.h"
-#include "utility.h"
 #include "vboot_api.h"
 
 /**
diff --git a/firmware/lib/include/utility.h b/firmware/lib/include/utility.h
deleted file mode 100644
index bca49828..00000000
--- a/firmware/lib/include/utility.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* Copyright (c) 2013 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.
- *
- * Helper functions/wrappers for memory allocations, manipulation and
- * comparison.
- */
-
-#ifndef VBOOT_REFERENCE_UTILITY_H_
-#define VBOOT_REFERENCE_UTILITY_H_
-
-#include "2common.h"
-#include "2sysincludes.h"
-#include "vboot_api.h"
-
-/*
- * Buffer size required to hold the longest possible output of Uint64ToString()
- * - that is, Uint64ToString(~0, 2).
- */
-#define UINT64_TO_STRING_MAX 65
-
-/**
- * Convert a value to a string in the specified radix (2=binary, 10=decimal,
- * 16=hex) and store it in <buf>, which is <bufsize> chars long.  If
- * <zero_pad_width>, left-pads the string to at least that width with '0'.
- * Returns the length of the stored string, not counting the terminating null.
- */
-uint32_t Uint64ToString(char *buf, uint32_t bufsize, uint64_t value,
-			uint32_t radix, uint32_t zero_pad_width);
-
-/**
- * Concatenate <src> onto <dest>, which has space for <destlen> characters
- * including the terminating null.  Note that <dest> will always be
- * null-terminated if <destlen> > 0.  Returns the number of characters used in
- * <dest>, not counting the terminating null.
- */
-uint32_t StrnAppend(char *dest, const char *src, uint32_t destlen);
-
-#endif  /* VBOOT_REFERENCE_UTILITY_H_ */
diff --git a/firmware/lib/tpm2_lite/marshaling.c b/firmware/lib/tpm2_lite/marshaling.c
index 90ce04ca..ef3606b9 100644
--- a/firmware/lib/tpm2_lite/marshaling.c
+++ b/firmware/lib/tpm2_lite/marshaling.c
@@ -6,7 +6,6 @@
 #include "2common.h"
 #include "2sysincludes.h"
 #include "tpm2_marshaling.h"
-#include "utility.h"
 
 static uint16_t tpm_tag;  /* Depends on the command type. */
 static int ph_disabled;   /* Platform hierarchy disabled. */
diff --git a/firmware/lib/tpm2_lite/tlcl.c b/firmware/lib/tpm2_lite/tlcl.c
index dca8facc..9bacbb6b 100644
--- a/firmware/lib/tpm2_lite/tlcl.c
+++ b/firmware/lib/tpm2_lite/tlcl.c
@@ -10,7 +10,7 @@
 #include "2sysincludes.h"
 #include "tlcl.h"
 #include "tpm2_marshaling.h"
-#include "utility.h"
+#include "vboot_api.h"
 
 /*
  * TODO(chromium:1032930): Originally accessed by including secdata_tpm.h.
diff --git a/firmware/lib/tpm_lite/mocked_tlcl.c b/firmware/lib/tpm_lite/mocked_tlcl.c
index d0f61201..838b5e7e 100644
--- a/firmware/lib/tpm_lite/mocked_tlcl.c
+++ b/firmware/lib/tpm_lite/mocked_tlcl.c
@@ -6,7 +6,6 @@
 #include "2sysincludes.h"
 #include "tlcl.h"
 #include "tlcl_internal.h"
-#include "utility.h"
 #include "vboot_api.h"
 
 uint32_t TlclLibInit(void)
diff --git a/firmware/lib/tpm_lite/tlcl.c b/firmware/lib/tpm_lite/tlcl.c
index b6685bf8..28c0bb63 100644
--- a/firmware/lib/tpm_lite/tlcl.c
+++ b/firmware/lib/tpm_lite/tlcl.c
@@ -21,7 +21,6 @@
 #include "tlcl.h"
 #include "tlcl_internal.h"
 #include "tlcl_structures.h"
-#include "utility.h"
 #include "vboot_api.h"
 
 /* Sets the size field of a TPM command. */
diff --git a/firmware/lib/utility_string.c b/firmware/lib/utility_string.c
deleted file mode 100644
index 299bbaa1..00000000
--- a/firmware/lib/utility_string.c
+++ /dev/null
@@ -1,72 +0,0 @@
-/* Copyright (c) 2013 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.
- *
- * String utility functions that need to be built as part of the firmware.
- */
-
-#include "2sysincludes.h"
-#include "utility.h"
-
-uint32_t Uint64ToString(char *buf, uint32_t bufsize, uint64_t value,
-			uint32_t radix, uint32_t zero_pad_width)
-{
-	char ibuf[UINT64_TO_STRING_MAX];
-	char *s;
-	uint32_t usedsize = 1;
-
-	if (!buf)
-		return 0;
-
-	/* Clear output buffer in case of error */
-	*buf = '\0';
-
-	/* Sanity-check input args */
-	if (radix < 2 || radix > 36 || zero_pad_width >= UINT64_TO_STRING_MAX)
-		return 0;
-
-	/* Start at end of string and work backwards */
-	s = ibuf + UINT64_TO_STRING_MAX - 1;
-	*(s) = '\0';
-	do {
-		int v = value % radix;
-		value /= radix;
-
-		*(--s) = (char)(v < 10 ? v + '0' : v + 'a' - 10);
-		if (++usedsize > bufsize)
-			return 0;  /* Result won't fit in buffer */
-	} while (value);
-
-	/* Zero-pad if necessary */
-	while (usedsize <= zero_pad_width) {
-		*(--s) = '0';
-		if (++usedsize > bufsize)
-			return 0;  /* Result won't fit in buffer */
-	}
-
-	/* Now copy the string back to the input buffer. */
-	memcpy(buf, s, usedsize);
-
-	/* Don't count the terminating null in the bytes used */
-	return usedsize - 1;
-}
-
-uint32_t StrnAppend(char *dest, const char *src, uint32_t destlen)
-{
-	uint32_t used = 0;
-
-	if (!dest || !src || !destlen)
-		return 0;
-
-	/* Skip past existing string in destination.*/
-	while (dest[used] && used < destlen - 1)
-		used++;
-
-	/* Now copy source */
-	while (*src && used < destlen - 1)
-		dest[used++] = *src++;
-
-	/* Terminate destination and return count of non-null characters */
-	dest[used] = 0;
-	return used;
-}
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 4f86046a..6111eee0 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -15,7 +15,6 @@
 #include "2sysincludes.h"
 #include "2ui.h"
 #include "load_kernel_fw.h"
-#include "utility.h"
 #include "vb2_common.h"
 #include "vboot_api.h"
 #include "vboot_kernel.h"
diff --git a/firmware/lib/vboot_audio.c b/firmware/lib/vboot_audio.c
index 20b14376..5e44606d 100644
--- a/firmware/lib/vboot_audio.c
+++ b/firmware/lib/vboot_audio.c
@@ -8,7 +8,6 @@
 #include "2common.h"
 #include "2misc.h"
 #include "2sysincludes.h"
-#include "utility.h"
 #include "vboot_api.h"
 #include "vboot_audio.h"
 
diff --git a/firmware/lib/vboot_display.c b/firmware/lib/vboot_display.c
index 258619fb..07e1da0a 100644
--- a/firmware/lib/vboot_display.c
+++ b/firmware/lib/vboot_display.c
@@ -10,7 +10,6 @@
 #include "2nvstorage.h"
 #include "2sha.h"
 #include "2sysincludes.h"
-#include "utility.h"
 #include "vboot_api.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
@@ -216,7 +215,12 @@ const char *RecoveryReasonString(uint8_t code)
 	return "Unknown or deprecated error code";
 }
 
-#define DEBUG_INFO_SIZE 512
+#define DEBUG_INFO_SIZE 1024
+#define DEBUG_INFO_APPEND(format, args...) do { \
+	if (used < DEBUG_INFO_SIZE) \
+		used += snprintf(buf + used, DEBUG_INFO_SIZE - used, format, \
+				 ## args); \
+} while (0)
 
 vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx)
 {
@@ -225,7 +229,7 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx)
 	struct vb2_workbuf wb;
 	char buf[DEBUG_INFO_SIZE] = "";
 	char sha1sum[VB2_SHA1_DIGEST_SIZE * 2 + 1];
-	uint32_t used = 0;
+	int32_t used = 0;
 	vb2_error_t ret;
 	uint32_t i;
 
@@ -238,83 +242,53 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx)
 		ret = vb2api_gbb_read_hwid(ctx, hwid, &size);
 		if (ret)
 			strcpy(hwid, "{INVALID}");
-		used += StrnAppend(buf + used, "HWID: ",
-				   DEBUG_INFO_SIZE - used);
-		used += StrnAppend(buf + used, hwid, DEBUG_INFO_SIZE - used);
+		DEBUG_INFO_APPEND("HWID: %s", hwid);
 	}
 
 	/* Add recovery reason and subcode */
 	i = vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE);
-	used += StrnAppend(buf + used,
-			"\nrecovery_reason: 0x", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       sd->recovery_reason, 16, 2);
-	used += StrnAppend(buf + used, " / 0x", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 16, 2);
-	used += StrnAppend(buf + used, "  ", DEBUG_INFO_SIZE - used);
-	used += StrnAppend(buf + used,
-			RecoveryReasonString(sd->recovery_reason),
-			DEBUG_INFO_SIZE - used);
+	DEBUG_INFO_APPEND("\nrecovery_reason: %#.2x / %#.2x  %s",
+			  sd->recovery_reason, i,
+			  RecoveryReasonString(sd->recovery_reason));
 
 	/* Add vb2_context and vb2_shared_data flags */
-	used += StrnAppend(buf + used, "\ncontext.flags: 0x",
-			   DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       ctx->flags, 16, 16);
-	used += StrnAppend(buf + used, "\nshared_data.flags: 0x",
-			   DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       sd->flags, 16, 8);
-	used += StrnAppend(buf + used, "\nshared_data.status: 0x",
-			   DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       sd->status, 16, 8);
-
-	/* Add raw contents of VbNvStorage */
-	used += StrnAppend(buf + used, "\nVbNv.raw:", DEBUG_INFO_SIZE - used);
+	DEBUG_INFO_APPEND("\ncontext.flags: %#.16" PRIx64, ctx->flags);
+	DEBUG_INFO_APPEND("\nshared_data.flags: %#.8x", sd->flags);
+	DEBUG_INFO_APPEND("\nshared_data.status: %#.8x", sd->status);
+
+	/* Add raw contents of nvdata */
+	DEBUG_INFO_APPEND("\nnvdata:");
+	if (vb2_nv_get_size(ctx) > 16)  /* Multi-line starts on next line */
+		DEBUG_INFO_APPEND("\n  ");
 	for (i = 0; i < vb2_nv_get_size(ctx); i++) {
-		used += StrnAppend(buf + used, " ", DEBUG_INFO_SIZE - used);
-		used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-				       ctx->nvdata[i], 16, 2);
+		/* Split into 16-byte blocks */
+		if (i > 0 && i % 16 == 0)
+			DEBUG_INFO_APPEND("\n  ");
+		DEBUG_INFO_APPEND(" %02x", ctx->nvdata[i]);
 	}
 
 	/* Add dev_boot_usb flag */
 	i = vb2_nv_get(ctx, VB2_NV_DEV_BOOT_USB);
-	used += StrnAppend(buf + used, "\ndev_boot_usb: ", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0);
+	DEBUG_INFO_APPEND("\ndev_boot_usb: %d", i);
 
 	/* Add dev_boot_legacy flag */
 	i = vb2_nv_get(ctx, VB2_NV_DEV_BOOT_LEGACY);
-	used += StrnAppend(buf + used,
-			"\ndev_boot_legacy: ", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0);
+	DEBUG_INFO_APPEND("\ndev_boot_legacy: %d", i);
 
 	/* Add dev_default_boot flag */
 	i = vb2_nv_get(ctx, VB2_NV_DEV_DEFAULT_BOOT);
-	used += StrnAppend(buf + used,
-			"\ndev_default_boot: ", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0);
+	DEBUG_INFO_APPEND("\ndev_default_boot: %d", i);
 
 	/* Add dev_boot_signed_only flag */
 	i = vb2_nv_get(ctx, VB2_NV_DEV_BOOT_SIGNED_ONLY);
-	used += StrnAppend(buf + used, "\ndev_boot_signed_only: ",
-			DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used, i, 10, 0);
+	DEBUG_INFO_APPEND("\ndev_boot_signed_only: %d", i);
 
 	/* Add TPM versions */
-	used += StrnAppend(buf + used,
-			   "\nTPM: fwver=0x", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       sd->fw_version_secdata, 16, 8);
-	used += StrnAppend(buf + used, " kernver=0x", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       sd->kernel_version_secdata, 16, 8);
+	DEBUG_INFO_APPEND("\nTPM: fwver=%#.8x kernver=%#.8x",
+			  sd->fw_version_secdata, sd->kernel_version_secdata);
 
 	/* Add GBB flags */
-	used += StrnAppend(buf + used,
-			   "\ngbb.flags: 0x", DEBUG_INFO_SIZE - used);
-	used += Uint64ToString(buf + used, DEBUG_INFO_SIZE - used,
-			       gbb->flags, 16, 8);
+	DEBUG_INFO_APPEND("\ngbb.flags: %#.8x", gbb->flags);
 
 	/* Add sha1sum for Root & Recovery keys */
 	{
@@ -323,10 +297,7 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx)
 		ret = vb2_gbb_read_root_key(ctx, &key, NULL, &wblocal);
 		if (!ret) {
 			FillInSha1Sum(sha1sum, key);
-			used += StrnAppend(buf + used, "\ngbb.rootkey: ",
-					   DEBUG_INFO_SIZE - used);
-			used += StrnAppend(buf + used, sha1sum,
-					   DEBUG_INFO_SIZE - used);
+			DEBUG_INFO_APPEND("\ngbb.rootkey: %s", sha1sum);
 		}
 	}
 
@@ -336,10 +307,7 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx)
 		ret = vb2_gbb_read_recovery_key(ctx, &key, NULL, &wblocal);
 		if (!ret) {
 			FillInSha1Sum(sha1sum, key);
-			used += StrnAppend(buf + used, "\ngbb.recovery_key: ",
-					   DEBUG_INFO_SIZE - used);
-			used += StrnAppend(buf + used, sha1sum,
-					   DEBUG_INFO_SIZE - used);
+			DEBUG_INFO_APPEND("\ngbb.recovery_key: %s", sha1sum);
 		}
 	}
 
@@ -349,13 +317,11 @@ vb2_error_t VbDisplayDebugInfo(struct vb2_context *ctx)
 		struct vb2_packed_key *key =
 			vb2_member_of(sd, sd->kernel_key_offset);
 		FillInSha1Sum(sha1sum, key);
-		used += StrnAppend(buf + used,
-				"\nkernel_subkey: ", DEBUG_INFO_SIZE - used);
-		used += StrnAppend(buf + used, sha1sum, DEBUG_INFO_SIZE - used);
+		DEBUG_INFO_APPEND("\nkernel_subkey: %s", sha1sum);
 	}
 
 	/* Make sure we finish with a newline */
-	used += StrnAppend(buf + used, "\n", DEBUG_INFO_SIZE - used);
+	DEBUG_INFO_APPEND("\n");
 
 	/* TODO: add more interesting data:
 	 * - Information on current disks */
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index 8118599d..7420a697 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -17,7 +17,6 @@
 #include "cgptlib_internal.h"
 #include "gpt_misc.h"
 #include "load_kernel_fw.h"
-#include "utility.h"
 #include "vb2_common.h"
 #include "vboot_api.h"
 #include "vboot_kernel.h"
diff --git a/firmware/lib/vboot_ui_legacy_clamshell.c b/firmware/lib/vboot_ui_legacy_clamshell.c
index 959981dd..c510d58f 100644
--- a/firmware/lib/vboot_ui_legacy_clamshell.c
+++ b/firmware/lib/vboot_ui_legacy_clamshell.c
@@ -13,7 +13,6 @@
 #include "2sysincludes.h"
 #include "load_kernel_fw.h"
 #include "tlcl.h"
-#include "utility.h"
 #include "vb2_common.h"
 #include "vboot_api.h"
 #include "vboot_audio.h"
diff --git a/firmware/lib/vboot_ui_legacy_menu.c b/firmware/lib/vboot_ui_legacy_menu.c
index b0cfb43b..48eaabe9 100644
--- a/firmware/lib/vboot_ui_legacy_menu.c
+++ b/firmware/lib/vboot_ui_legacy_menu.c
@@ -12,7 +12,6 @@
 #include "2secdata.h"
 #include "2sysincludes.h"
 #include "load_kernel_fw.h"
-#include "utility.h"
 #include "vb2_common.h"
 #include "vboot_api.h"
 #include "vboot_audio.h"