[PATCH v2 04/12] sysinfo: Make sysinfo_get_str() behave like snprintf()

Marek Behún kabel at kernel.org
Thu Nov 4 00:23:24 CET 2021


From: Marek Behún <marek.behun at nic.cz>

Currently sysinfo_get_str() returns 0 if a string is filled in the
given buffer, and otherwise gives no simple mechanism to determine
actual string length.

One implementation returns -ENOSPC if buffer is not large enough.

Change the behaviour of the function to that of snprintf(): i.e. the
buffer is always filled in as much as possible if the string exists, and
the function returns the actual length of the string (excluding the
terminating NULL-byte).

Signed-off-by: Marek Behún <marek.behun at nic.cz>
---
 board/google/chromebook_coral/coral.c | 13 ++++---------
 common/board_info.c                   |  2 +-
 drivers/sysinfo/gpio.c                |  2 +-
 drivers/sysinfo/rcar3.c               |  2 +-
 drivers/sysinfo/sandbox.c             |  5 +++--
 include/sysinfo.h                     | 16 ++++++++++++----
 lib/smbios.c                          |  2 +-
 test/dm/sysinfo-gpio.c                | 12 ++++++------
 test/dm/sysinfo.c                     | 12 ++++++------
 9 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c
index 53c5171d02..124fc49998 100644
--- a/board/google/chromebook_coral/coral.c
+++ b/board/google/chromebook_coral/coral.c
@@ -155,10 +155,8 @@ static int coral_get_str(struct udevice *dev, int id, size_t size, char *val)
 
 		if (ret < 0)
 			return ret;
-		if (size < 15)
-			return -ENOSPC;
-		sprintf(val, "rev%d", ret);
-		break;
+
+		return snprintf(val, size, "rev%d", ret);
 	}
 	case SYSINFO_ID_BOARD_MODEL: {
 		int mem_config, sku_config;
@@ -173,15 +171,12 @@ static int coral_get_str(struct udevice *dev, int id, size_t size, char *val)
 			log_warning("Unable to read skuconfig (err=%d)\n", ret);
 		sku_config = ret;
 		model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
-		snprintf(val, size, "%s (memconfig %d, SKU %d)", model,
-			 mem_config, sku_config);
-		break;
+		return snprintf(val, size, "%s (memconfig %d, SKU %d)", model,
+				mem_config, sku_config);
 	}
 	default:
 		return -ENOENT;
 	}
-
-	return 0;
 }
 
 int chromeos_get_gpio(const struct udevice *dev, const char *prop,
diff --git a/common/board_info.c b/common/board_info.c
index e0f2d93922..fd7bd1deea 100644
--- a/common/board_info.c
+++ b/common/board_info.c
@@ -43,7 +43,7 @@ int __weak show_board_info(void)
 		}
 
 		/* Fail back to the main 'model' if available */
-		if (ret)
+		if (ret <= 0)
 			model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
 		else
 			model = str;
diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c
index 1d7f050998..8a9238b589 100644
--- a/drivers/sysinfo/gpio.c
+++ b/drivers/sysinfo/gpio.c
@@ -79,7 +79,7 @@ static int sysinfo_gpio_get_str(struct udevice *dev, int id, size_t size, char *
 
 		strncpy(val, name, size);
 		val[size - 1] = '\0';
-		return 0;
+		return strlen(name);
 	} default:
 		return -EINVAL;
 	};
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index c2f4ddfbbe..3bfd9bc39d 100644
--- a/drivers/sysinfo/rcar3.c
+++ b/drivers/sysinfo/rcar3.c
@@ -48,7 +48,7 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *
 	case SYSINFO_ID_BOARD_MODEL:
 		strncpy(val, priv->boardmodel, size);
 		val[size - 1] = '\0';
-		return 0;
+		return strlen(priv->boardmodel);
 	default:
 		return -EINVAL;
 	};
diff --git a/drivers/sysinfo/sandbox.c b/drivers/sysinfo/sandbox.c
index d270a26aa4..01c845e310 100644
--- a/drivers/sysinfo/sandbox.c
+++ b/drivers/sysinfo/sandbox.c
@@ -73,8 +73,9 @@ int sysinfo_sandbox_get_str(struct udevice *dev, int id, size_t size, char *val)
 	switch (id) {
 	case STR_VACATIONSPOT:
 		/* Picks a vacation spot depending on i1 and i2 */
-		snprintf(val, size, vacation_spots[index]);
-		return 0;
+		strncpy(val, vacation_spots[index], size);
+		val[size - 1] = '\0';
+		return strlen(vacation_spots[index]);
 	}
 
 	return -ENOENT;
diff --git a/include/sysinfo.h b/include/sysinfo.h
index b140d742e9..6031aec578 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -95,10 +95,15 @@ struct sysinfo_ops {
 	 * @dev:	The sysinfo instance to gather the data.
 	 * @id:		A unique identifier for the string value to be read.
 	 * @size:	The size of the buffer to receive the string data.
+	 *		If the buffer is not large enough to contain the whole
+	 *		string, the string must be trimmed to fit in the
+	 *		buffer including the terminating NULL-byte.
 	 * @val:	Pointer to a buffer that receives the value read.
 	 *
-	 * Return: 0 if OK, -ve on error.
+	 * Return: Actual length of the string excluding the terminating
+	 *	   NULL-byte if OK, -ve on error.
 	 */
+
 	int (*get_str)(struct udevice *dev, int id, size_t size, char *val);
 
 	/**
@@ -164,11 +169,14 @@ int sysinfo_get_int(struct udevice *dev, int id, int *val);
  *		     hardware setup.
  * @dev:	The sysinfo instance to gather the data.
  * @id:		A unique identifier for the string value to be read.
- * @size:	The size of the buffer to receive the string data.
+ * @size:	The size of the buffer to receive the string data. If the buffer
+ *		is not large enough to contain the whole string, the string will
+ *		be trimmed to fit in the buffer including the terminating
+ *		NULL-byte.
  * @val:	Pointer to a buffer that receives the value read.
  *
- * Return: 0 if OK, -EPERM if called before sysinfo_detect(), else -ve on
- * error.
+ * Return: Actual length of the string excluding the terminating NULL-byte if
+ *	   OK, -EPERM if called before sysinfo_detect(), else -ve on error.
  */
 int sysinfo_get_str(struct udevice *dev, int id, size_t size, char *val);
 
diff --git a/lib/smbios.c b/lib/smbios.c
index d7f4999e8b..0e55a0e49f 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -144,7 +144,7 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
 		int ret;
 
 		ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val);
-		if (!ret)
+		if (ret >= 0)
 			return smbios_add_string(ctx, val);
 	}
 	if (IS_ENABLED(CONFIG_OF_CONTROL)) {
diff --git a/test/dm/sysinfo-gpio.c b/test/dm/sysinfo-gpio.c
index 2e494b3f34..2136b2000d 100644
--- a/test/dm/sysinfo-gpio.c
+++ b/test/dm/sysinfo-gpio.c
@@ -32,8 +32,8 @@ static int dm_test_sysinfo_gpio(struct unit_test_state *uts)
 	ut_assertok(sysinfo_detect(sysinfo));
 	ut_assertok(sysinfo_get_int(sysinfo, SYSINFO_ID_BOARD_MODEL, &val));
 	ut_asserteq(19, val);
-	ut_assertok(sysinfo_get_str(sysinfo, SYSINFO_ID_BOARD_MODEL, sizeof(buf),
-				    buf));
+	ut_asserteq(5, sysinfo_get_str(sysinfo, SYSINFO_ID_BOARD_MODEL, sizeof(buf),
+				       buf));
 	ut_asserteq_str("rev_a", buf);
 
 	/*
@@ -46,8 +46,8 @@ static int dm_test_sysinfo_gpio(struct unit_test_state *uts)
 	ut_assertok(sysinfo_detect(sysinfo));
 	ut_assertok(sysinfo_get_int(sysinfo, SYSINFO_ID_BOARD_MODEL, &val));
 	ut_asserteq(5, val);
-	ut_assertok(sysinfo_get_str(sysinfo, SYSINFO_ID_BOARD_MODEL, sizeof(buf),
-				    buf));
+	ut_asserteq(3, sysinfo_get_str(sysinfo, SYSINFO_ID_BOARD_MODEL, sizeof(buf),
+				       buf));
 	ut_asserteq_str("foo", buf);
 
 	/*
@@ -60,8 +60,8 @@ static int dm_test_sysinfo_gpio(struct unit_test_state *uts)
 	ut_assertok(sysinfo_detect(sysinfo));
 	ut_assertok(sysinfo_get_int(sysinfo, SYSINFO_ID_BOARD_MODEL, &val));
 	ut_asserteq(15, val);
-	ut_assertok(sysinfo_get_str(sysinfo, SYSINFO_ID_BOARD_MODEL, sizeof(buf),
-				    buf));
+	ut_asserteq(7, sysinfo_get_str(sysinfo, SYSINFO_ID_BOARD_MODEL, sizeof(buf),
+				       buf));
 	ut_asserteq_str("unknown", buf);
 
 	return 0;
diff --git a/test/dm/sysinfo.c b/test/dm/sysinfo.c
index 96b3a8ebab..488412ab14 100644
--- a/test/dm/sysinfo.c
+++ b/test/dm/sysinfo.c
@@ -34,8 +34,8 @@ static int dm_test_sysinfo(struct unit_test_state *uts)
 				     &called_detect));
 	ut_assert(called_detect);
 
-	ut_assertok(sysinfo_get_str(sysinfo, STR_VACATIONSPOT, sizeof(str),
-				    str));
+	ut_asserteq(6, sysinfo_get_str(sysinfo, STR_VACATIONSPOT, sizeof(str),
+				       str));
 	ut_assertok(strcmp(str, "R'lyeh"));
 
 	ut_assertok(sysinfo_get_int(sysinfo, INT_TEST1, &i));
@@ -44,8 +44,8 @@ static int dm_test_sysinfo(struct unit_test_state *uts)
 	ut_assertok(sysinfo_get_int(sysinfo, INT_TEST2, &i));
 	ut_asserteq(100, i);
 
-	ut_assertok(sysinfo_get_str(sysinfo, STR_VACATIONSPOT, sizeof(str),
-				    str));
+	ut_asserteq(7, sysinfo_get_str(sysinfo, STR_VACATIONSPOT, sizeof(str),
+				       str));
 	ut_assertok(strcmp(str, "Carcosa"));
 
 	ut_assertok(sysinfo_get_int(sysinfo, INT_TEST1, &i));
@@ -54,8 +54,8 @@ static int dm_test_sysinfo(struct unit_test_state *uts)
 	ut_assertok(sysinfo_get_int(sysinfo, INT_TEST2, &i));
 	ut_asserteq(99, i);
 
-	ut_assertok(sysinfo_get_str(sysinfo, STR_VACATIONSPOT, sizeof(str),
-				    str));
+	ut_asserteq(7, sysinfo_get_str(sysinfo, STR_VACATIONSPOT, sizeof(str),
+				       str));
 	ut_assertok(strcmp(str, "Yuggoth"));
 
 	return 0;
-- 
2.32.0



More information about the U-Boot mailing list