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

Marek Behún
2021-11-03T23:24:33Z

From: Marek Behún <marek.behun@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@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