printf: Convert %b to %pb

In order to turn on compile-time printf format checking,
non-standard specifiers like %b (binary) must be removed. Convert
that into %pb, which takes a pointer to a structure containing the
value to print, and how many bits to print. Use the convenience
macro BINARY_VALUE() to package these values up into a struct whose
pointer is passed to printf().

Technically this is slightly more limited functionality than we used
to support given all the possible flags, field width, and precision.
However every existing instance in our codebase was using %0NNb, where
NN is some number. If more variants are needed, the parameters structure
can be expanded in the future.

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

Change-Id: I8ef995dcf97af688fbca98ab6ff59b84092b69e3
Signed-off-by: Evan Green <evgreen@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1733100
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>

9 files changed, 66 insertions, 27 deletions

diff --git a/common/charger.c b/common/charger.c
index 87d2f8e0ce..7ee80a9c23 100644
--- a/common/charger.c
+++ b/common/charger.c
@@ -128,7 +128,7 @@ void print_charger_debug(void)
 	/* option */
 	print_item_name("Option:");
 	if (check_print_error(charger_get_option(&d)))
-		ccprintf("%016b (0x%04x)\n", d, d);
+		ccprintf("%pb (0x%04x)\n", BINARY_VALUE(d, 16), d);
 
 	/* manufacturer id */
 	print_item_name("Man id:");
diff --git a/common/printf.c b/common/printf.c
index 777c78b471..4da70653ee 100644
--- a/common/printf.c
+++ b/common/printf.c
@@ -230,6 +230,7 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 			}
 
 			if (c == 'p') {
+				c = -1;
 				ptrspec = *format++;
 				ptrval = va_arg(args, void *);
 				/* %pT - print a timestamp. */
@@ -271,10 +272,21 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 				} else if (ptrspec == 'P') {
 					/* %pP - Print a raw pointer. */
 					v = (unsigned long)ptrval;
+					base = 16;
 					if (sizeof(unsigned long) ==
 					    sizeof(uint64_t))
 						flags |= PF_64BIT;
 
+				} else if (ptrspec == 'b') {
+					/* %pb - Print a binary integer */
+					struct binary_print_params *binary =
+						ptrval;
+
+					v = binary->value;
+					pad_width = binary->count;
+					flags |= PF_PADZERO;
+					base = 2;
+
 				} else {
 					return EC_ERROR_INVAL;
 				}
@@ -316,11 +328,11 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context,
 				break;
 			case 'X':
 			case 'x':
-			case 'p':
 				base = 16;
 				break;
-			case 'b':
-				base = 2;
+
+			/* Int passthrough for pointers. */
+			case -1:
 				break;
 			default:
 				format = error_str;
diff --git a/driver/charger/sy21612.c b/driver/charger/sy21612.c
index c8751ce208..0a1b122d28 100644
--- a/driver/charger/sy21612.c
+++ b/driver/charger/sy21612.c
@@ -199,7 +199,7 @@ static int command_sy21612(int argc, char **argv)
 		if (rv)
 			ccprintf(" x (%d)\n", rv);
 		else
-			ccprintf("%02x - %08b\n", val, val);
+			ccprintf("%02x - %pb\n", val, BINARY_VALUE(val, 8));
 	}
 
 	ccprintf("vbat voltage: %d mV\n", sy21612_get_vbat_voltage());
diff --git a/driver/temp_sensor/adt7481.c b/driver/temp_sensor/adt7481.c
index df6ff4bb7c..cbd32e5cd5 100644
--- a/driver/temp_sensor/adt7481.c
+++ b/driver/temp_sensor/adt7481.c
@@ -248,16 +248,16 @@ static int print_status(void)
 	ccprintf("\n");
 
 	if (raw_read8(ADT7481_STATUS1_R, &value) == EC_SUCCESS)
-		ccprintf("STATUS1:  %08b\n", value);
+		ccprintf("STATUS1:  %pb\n", BINARY_VALUE(value, 8));
 
 	if (raw_read8(ADT7481_STATUS2_R, &value) == EC_SUCCESS)
-		ccprintf("STATUS2:  %08b\n", value);
+		ccprintf("STATUS2:  %pb\n", BINARY_VALUE(value, 8));
 
 	if (raw_read8(ADT7481_CONFIGURATION1_R, &value) == EC_SUCCESS)
-		ccprintf("CONFIG1: %08b\n", value);
+		ccprintf("CONFIG1: %pb\n", BINARY_VALUE(value, 8));
 
 	if (raw_read8(ADT7481_CONFIGURATION2, &value) == EC_SUCCESS)
-		ccprintf("CONFIG2: %08b\n", value);
+		ccprintf("CONFIG2: %pb\n", BINARY_VALUE(value, 8));
 
 	return EC_SUCCESS;
 }
@@ -307,7 +307,8 @@ static int command_adt7481(int argc, char **argv)
 		rv = raw_read8(offset, &data);
 		if (rv < 0)
 			return rv;
-		ccprintf("Byte at offset 0x%02x is %08b\n", offset, data);
+		ccprintf("Byte at offset 0x%02x is %pb\n",
+			 offset, BINARY_VALUE(data, 8));
 		return rv;
 	}
 
diff --git a/driver/temp_sensor/g78x.c b/driver/temp_sensor/g78x.c
index 935586b8fa..aef13d3d68 100644
--- a/driver/temp_sensor/g78x.c
+++ b/driver/temp_sensor/g78x.c
@@ -165,15 +165,15 @@ static int print_status(void)
 	ccprintf("\n");
 
 	if (raw_read8(G78X_STATUS, &value) == EC_SUCCESS)
-		ccprintf("STATUS:  %08b\n", value);
+		ccprintf("STATUS:  %pb\n", BINARY_VALUE(value, 8));
 
 #ifdef CONFIG_TEMP_SENSOR_G782
 	if (raw_read8(G78X_STATUS1, &value) == EC_SUCCESS)
-		ccprintf("STATUS1: %08b\n", value);
+		ccprintf("STATUS1: %pb\n", BINARY_VALUE(value, 8));
 #endif
 
 	if (raw_read8(G78X_CONFIGURATION_R, &value) == EC_SUCCESS)
-		ccprintf("CONFIG:  %08b\n", value);
+		ccprintf("CONFIG:  %pb\n", BINARY_VALUE(value, 8));
 
 	return EC_SUCCESS;
 }
@@ -207,7 +207,8 @@ static int command_g78x(int argc, char **argv)
 		rv = raw_read8(offset, &data);
 		if (rv < 0)
 			return rv;
-		ccprintf("Byte at offset 0x%02x is %08b\n", offset, data);
+		ccprintf("Byte at offset 0x%02x is %pb\n",
+			 offset, BINARY_VALUE(data, 8));
 		return rv;
 	}
 
diff --git a/driver/temp_sensor/tmp411.c b/driver/temp_sensor/tmp411.c
index e65c9eae8c..ef22052da8 100644
--- a/driver/temp_sensor/tmp411.c
+++ b/driver/temp_sensor/tmp411.c
@@ -232,10 +232,10 @@ static int print_status(void)
 	ccprintf("\n");
 
 	if (raw_read8(TMP411_STATUS_R, &value) == EC_SUCCESS)
-		ccprintf("STATUS:  %08b\n", value);
+		ccprintf("STATUS:  %pb\n", BINARY_VALUE(value, 8));
 
 	if (raw_read8(TMP411_CONFIGURATION1_R, &value) == EC_SUCCESS)
-		ccprintf("CONFIG1: %08b\n", value);
+		ccprintf("CONFIG1: %pb\n", BINARY_VALUE(value, 8));
 
 	return EC_SUCCESS;
 }
@@ -285,7 +285,8 @@ static int command_tmp411(int argc, char **argv)
 		rv = raw_read8(offset, &data);
 		if (rv < 0)
 			return rv;
-		ccprintf("Byte at offset 0x%02x is %08b\n", offset, data);
+		ccprintf("Byte at offset 0x%02x is %pb\n",
+			 offset, BINARY_VALUE(data, 8));
 		return rv;
 	}
 
diff --git a/driver/temp_sensor/tmp432.c b/driver/temp_sensor/tmp432.c
index 1b95886c63..9f76e74660 100644
--- a/driver/temp_sensor/tmp432.c
+++ b/driver/temp_sensor/tmp432.c
@@ -290,13 +290,13 @@ static int print_status(void)
 	ccprintf("\n");
 
 	if (raw_read8(TMP432_STATUS, &value) == EC_SUCCESS)
-		ccprintf("STATUS:  %08b\n", value);
+		ccprintf("STATUS:  %pb\n", BINARY_VALUE(value, 8));
 
 	if (raw_read8(TMP432_CONFIGURATION1_R, &value) == EC_SUCCESS)
-		ccprintf("CONFIG1: %08b\n", value);
+		ccprintf("CONFIG1: %pb\n", BINARY_VALUE(value, 8));
 
 	if (raw_read8(TMP432_CONFIGURATION2_R, &value) == EC_SUCCESS)
-		ccprintf("CONFIG2: %08b\n", value);
+		ccprintf("CONFIG2: %pb\n", BINARY_VALUE(value, 8));
 
 	return EC_SUCCESS;
 }
@@ -347,7 +347,8 @@ static int command_tmp432(int argc, char **argv)
 		rv = raw_read8(offset, &data);
 		if (rv < 0)
 			return rv;
-		ccprintf("Byte at offset 0x%02x is %08b\n", offset, data);
+		ccprintf("Byte at offset 0x%02x is %pb\n",
+			 offset, BINARY_VALUE(data, 8));
 		return rv;
 	}
 
diff --git a/include/console.h b/include/console.h
index dc15e5b6d3..9246e92b25 100644
--- a/include/console.h
+++ b/include/console.h
@@ -26,6 +26,21 @@ struct hex_buffer_params {
 	.size = (_size) \
 })
 
+/*
+ * Define parameters to printing in binary: the value to print, and the number
+ * of digits to print.
+ */
+
+struct binary_print_params {
+	unsigned int value;
+	uint8_t count;
+};
+
+#define BINARY_VALUE(_value, _count) (&(const struct binary_print_params){ \
+	.value = (_value), \
+	.count = (_count) \
+})
+
 #define PRINTF_TIMESTAMP_NOW NULL
 
 /* Console command; used by DECLARE_CONSOLE_COMMAND macro. */
diff --git a/test/printf.c b/test/printf.c
index c758d9bccb..97bb00c4be 100644
--- a/test/printf.c
+++ b/test/printf.c
@@ -186,30 +186,38 @@ test_static int test_vsnprintf_int(void)
 	T(expect_success("5e",        "%x",     0X5E));
 	T(expect_success("5E",        "%X",     0X5E));
 
-	T(expect_success("0",         "%b",     0));
-	T(expect_success("1011110",   "%b",     0X5E));
-
 	return EC_SUCCESS;
 }
 
 test_static int test_vsnprintf_pointers(void)
 {
 	void *ptr = (void *)0x55005E00;
+	unsigned int val = 0;
 
 	T(expect_success("55005e00",  "%pP",     ptr));
 	T(expect_success(err_str,     "%P",      ptr));
 	/* %p by itself is invalid */
 	T(expect(EC_ERROR_INVAL, NO_BYTES_TOUCHED,
-		 /* given -1 as output size limit */
 		 false, -1, "%p"));
 	/* %p with an unknown suffix is invalid */
 	T(expect(EC_ERROR_INVAL, NO_BYTES_TOUCHED,
-		 /* given -1 as output size limit */
 		 false, -1, "%p "));
 	/* %p with an unknown suffix is invalid */
 	T(expect(EC_ERROR_INVAL, NO_BYTES_TOUCHED,
-		 /* given -1 as output size limit */
 		 false, -1, "%pQ"));
+
+	/* Test %pb, binary format */
+	T(expect_success("0",         "%pb",     BINARY_VALUE(val, 0)));
+	val = 0x5E;
+	T(expect_success("1011110",   "%pb",     BINARY_VALUE(val, 0)));
+	T(expect_success("0000000001011110", "%pb", BINARY_VALUE(val, 16)));
+	val = 0x12345678;
+	T(expect_success("10010001101000101011001111000", "%pb",
+			 BINARY_VALUE(val, 0)));
+	val = 0xFEDCBA90;
+	/* Test a number that makes the longest string possible */
+	T(expect_success("11111110110111001011101010010000", "%pb",
+			 BINARY_VALUE(val, 0)));
 	return EC_SUCCESS;
 }