printf: Convert %h to %ph

In order to make printf more standard, use %ph. Pass a pointer to
a struct describing the buffer, including its size. Add a convenience
macro so that conversion between the old style and new style is purely
mechanical. The old style of %h cannot be converted directly to %ph as-is
because the C standard doesn't allow flags, precision, or field width on
%p.

Ultimately the goal is to enable compile-time printf format checking.
This gets us one step closer to that.

BUG=chromium:984041
TEST=make -j buildall
BRANCH=None

Cq-Depend:chrome-internal:1559798,chrome-internal:1560598
Change-Id: I9c0ca124a048314c9b62d64bd55b36be55034e0e
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1730605

7 files changed, 86 insertions, 56 deletions

diff --git a/common/ccd_config.c b/common/ccd_config.c
index fc9409a13f..91232df295 100644
--- a/common/ccd_config.c
+++ b/common/ccd_config.c
@@ -714,7 +714,7 @@ static int command_ccd_info(void)
 	ccprintf("Password: %s\n", raw_has_password() ? "set" : "none");
 	ccprintf("Flags: 0x%06x\n", raw_get_flags());
 
-	ccprintf("Capabilities: %.8h\n", config.capabilities);
+	ccprintf("Capabilities: %ph\n", HEX_BUF(config.capabilities, 8));
 	for (i = 0; i < CCD_CAP_COUNT; i++) {
 		int c = raw_get_cap(i, 0);
 
diff --git a/common/host_command.c b/common/host_command.c
index be178f7b6c..eff1fac56b 100644
--- a/common/host_command.c
+++ b/common/host_command.c
@@ -666,8 +666,9 @@ static void host_command_debug_request(struct host_cmd_handler_args *args)
 	}
 
 	if (hcdebug >= HCDEBUG_PARAMS && args->params_size)
-		CPRINTS("HC 0x%02x.%d:%.*h", args->command,
-			args->version, args->params_size, args->params);
+		CPRINTS("HC 0x%02x.%d:%ph", args->command,
+			args->version,
+			HEX_BUF(args->params, args->params_size));
 	else
 		CPRINTS("HC 0x%02x", args->command);
 }
@@ -711,8 +712,8 @@ uint16_t host_command_process(struct host_cmd_handler_args *args)
 		CPRINTS("HC 0x%02x err %d", args->command, rv);
 
 	if (hcdebug >= HCDEBUG_PARAMS && args->response_size)
-		CPRINTS("HC resp:%.*h", args->response_size,
-			args->response);
+		CPRINTS("HC resp:%ph",
+			HEX_BUF(args->response, args->response_size));
 
 	return rv;
 }
@@ -892,7 +893,8 @@ static int command_host_command(int argc, char **argv)
 	if (res != EC_RES_SUCCESS)
 		ccprintf("Command returned %d\n", res);
 	else if (args.response_size)
-		ccprintf("Response: %.*h\n", args.response_size, cmd_params);
+		ccprintf("Response: %ph\n",
+			 HEX_BUF(cmd_params, args.response_size));
 	else
 		ccprintf("Command succeeded; no response.\n");
 
diff --git a/common/i2c_master.c b/common/i2c_master.c
index ea33b6f533..091ea7f423 100644
--- a/common/i2c_master.c
+++ b/common/i2c_master.c
@@ -1145,7 +1145,7 @@ static int command_i2cxfer(int argc, char **argv)
 			      (uint8_t *)&offset, 1, data, v);
 
 		if (!rv)
-			ccprintf("Data: %.*h\n", v, data);
+			ccprintf("Data: %ph\n", HEX_BUF(data, v));
 
 	} else if (strcasecmp(argv[1], "w") == 0) {
 		/* 8-bit write */
diff --git a/common/peci.c b/common/peci.c
index b4422de05e..e0f03c95dd 100644
--- a/common/peci.c
+++ b/common/peci.c
@@ -140,7 +140,7 @@ static int peci_cmd(int argc, char **argv)
 		ccprintf("PECI transaction error\n");
 		return EC_ERROR_UNKNOWN;
 	}
-	ccprintf("PECI read data: %.*h\n", peci.r_len, r_buf);
+	ccprintf("PECI read data: %ph\n", HEX_BUF(r_buf, peci.r_len));
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(peci, peci_cmd,
diff --git a/common/printf.c b/common/printf.c
index 89ee004452..777c78b471 100644
--- a/common/printf.c
+++ b/common/printf.c
@@ -5,6 +5,7 @@
 
 /* Printf-like functionality for Chrome EC */
 
+#include "console.h"
 #include "printf.h"
 #include "timer.h"
 #include "util.h"
@@ -18,6 +19,7 @@ static inline int divmod(uint64_t *n, int d)
 {
 	return uint64divmod(n, d);
 }
+
 #else /* CONFIG_DEBUG_PRINTF */
 /* if we are optimizing for size, remove the 64-bit support */
 #define NO_UINT64_SUPPORT
@@ -48,7 +50,55 @@ static int hexdigit(int c)
 #define PF_LEFT		BIT(0)  /* Left-justify */
 #define PF_PADZERO	BIT(1)  /* Pad with 0's not spaces */
 #define PF_SIGN		BIT(2)  /* Add sign (+) for a positive number */
+
+/* Deactivate the PF_64BIT flag is 64-bit support is disabled. */
+#ifdef NO_UINT64_SUPPORT
+#define PF_64BIT	0
+#else
 #define PF_64BIT	BIT(3)  /* Number is 64-bit */
+#endif
+
+/*
+ * Print the buffer as a string of bytes in hex.
+ * Returns 0 on success or an error on failure.
+ */
+static int print_hex_buffer(int (*addchar)(void *context, int c),
+			    void *context, const char *vstr, int precision,
+			    int pad_width, int flags)
+
+{
+
+	/*
+	 * Divide pad_width instead of multiplying precision to avoid overflow
+	 * error in the condition. The "/2" and "2*" can be optimized by
+	 * the compiler.
+	 */
+
+	if ((pad_width / 2) >= precision)
+		pad_width -= 2 * precision;
+	else
+		pad_width = 0;
+
+	while (pad_width > 0 && !(flags & PF_LEFT)) {
+		if (addchar(context, flags & PF_PADZERO ? '0' : ' '))
+			return EC_ERROR_OVERFLOW;
+		pad_width--;
+	}
+
+	for (; precision; precision--, vstr++) {
+		if (addchar(context, hexdigit(*vstr >> 4)) ||
+		    addchar(context, hexdigit(*vstr)))
+			return EC_ERROR_OVERFLOW;
+	}
+
+	while (pad_width > 0 && (flags & PF_LEFT)) {
+		if (addchar(context, ' '))
+			return EC_ERROR_OVERFLOW;
+		pad_width--;
+	}
+
+	return EC_SUCCESS;
+}
 
 int vfnprintf(int (*addchar)(void *context, int c), void *context,
 	      const char *format, va_list args)
@@ -162,54 +212,14 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 			vstr = va_arg(args, char *);
 			if (vstr == NULL)
 				vstr = "(NULL)";
-		} else if (c == 'h') {
-			/* Hex dump output */
-			vstr = va_arg(args, char *);
-
-			if (precision < 0) {
-				/* Hex dump requires precision */
-				format = error_str;
-				continue;
-			}
-
-			/*
-			 * Divide pad_width instead of multiplying
-			 * precision to avoid overflow error
-			 * in the condition.
-			 * The "/2" and "2*" can be optimized by
-			 * the compiler.
-			 */
-			if ((pad_width/2) >= precision)
-				pad_width -= 2*precision;
-			else
-				pad_width = 0;
-
-			while (pad_width > 0 && !(flags & PF_LEFT)) {
-				if (addchar(context,
-					    flags & PF_PADZERO ? '0' : ' '))
-					return EC_ERROR_OVERFLOW;
-				pad_width--;
-			}
-			for (; precision; precision--, vstr++) {
-				if (addchar(context, hexdigit(*vstr >> 4)) ||
-				    addchar(context, hexdigit(*vstr)))
-					return EC_ERROR_OVERFLOW;
-			}
-			while (pad_width > 0 && (flags & PF_LEFT)) {
-				if (addchar(context, ' '))
-					return EC_ERROR_OVERFLOW;
-				pad_width--;
-			}
 
-			continue;
 		} else {
 			int base = 10;
 #ifdef NO_UINT64_SUPPORT
 			uint32_t v;
-
-			v = va_arg(args, uint32_t);
-#else /* NO_UINT64_SUPPORT */
+#else
 			uint64_t v;
+#endif
 			int ptrspec;
 			void *ptrval;
 
@@ -223,7 +233,8 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 				ptrspec = *format++;
 				ptrval = va_arg(args, void *);
 				/* %pT - print a timestamp. */
-				if (ptrspec == 'T') {
+				if (ptrspec == 'T' &&
+				    !IS_ENABLED(NO_UINT64_SUPPORT)) {
 					flags |= PF_64BIT;
 					/* NULL uses the current time. */
 					if (ptrval == NULL)
@@ -239,8 +250,26 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 						v /= 1000;
 					}
 
+				} else if (ptrspec == 'h') {
+					/* %ph - Print a hex byte buffer. */
+					struct hex_buffer_params *hexbuf =
+						ptrval;
+					int rc;
+
+					rc = print_hex_buffer(addchar,
+							      context,
+							      hexbuf->buffer,
+							      hexbuf->size,
+							      0,
+							      0);
+
+					if (rc != EC_SUCCESS)
+						return rc;
+
+					continue;
+
 				} else if (ptrspec == 'P') {
-					/* Print a raw pointer. */
+					/* %pP - Print a raw pointer. */
 					v = (unsigned long)ptrval;
 					if (sizeof(unsigned long) ==
 					    sizeof(uint64_t))
@@ -255,7 +284,6 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 			} else {
 				v = va_arg(args, uint32_t);
 			}
-#endif
 
 			switch (c) {
 #ifdef CONFIG_PRINTF_LEGACY_LI_FORMAT
diff --git a/common/spi_commands.c b/common/spi_commands.c
index 9fdc721d40..1a70a5be82 100644
--- a/common/spi_commands.c
+++ b/common/spi_commands.c
@@ -46,7 +46,7 @@ static int command_spixfer(int argc, char **argv)
 		rv = spi_transaction(&spi_devices[dev_id], &cmd, 1, data, v);
 
 		if (!rv)
-			ccprintf("Data: %.*h\n", v, data);
+			ccprintf("Data: %ph\n", HEX_BUF(data, v));
 
 	} else if (strcasecmp(argv[1], "w") == 0) {
 		/* 8-bit write */
diff --git a/common/vboot_hash.c b/common/vboot_hash.c
index 4d12af7c7b..0642e09e3c 100644
--- a/common/vboot_hash.c
+++ b/common/vboot_hash.c
@@ -141,7 +141,7 @@ static void vboot_hash_next_chunk(void)
 	if (curr_pos >= data_size) {
 		/* Store the final hash */
 		hash = SHA256_final(&ctx);
-		CPRINTS("hash done %.*h", SHA256_PRINT_SIZE, hash);
+		CPRINTS("hash done %ph", HEX_BUF(hash, SHA256_PRINT_SIZE));
 
 		in_progress = 0;
 
@@ -331,7 +331,7 @@ static int command_hash(int argc, char **argv)
 		else if (in_progress)
 			ccprintf("(in progress)\n");
 		else if (hash)
-			ccprintf("%.*h\n", SHA256_DIGEST_SIZE, hash);
+			ccprintf("%ph\n", HEX_BUF(hash, SHA256_DIGEST_SIZE));
 		else
 			ccprintf("(invalid)\n");