diff mbox series

[v2,06/12] sysinfo: Add get_str_list() method

Message ID 20211103232332.2737-7-kabel@kernel.org
State Changes Requested
Delegated to: Simon Glass
Headers show
Series Board specific runtime determined default env | expand

Commit Message

Marek Behún Nov. 3, 2021, 11:23 p.m. UTC
From: Marek Behún <marek.behun@nic.cz>

Add get_str_list() method to sysinfo operations.

The get_str_list() method is similar to get_str(), but receives one
additional argument, @idx, and fills in the @idx-th string from a given
list.

Add sandbox implementation together with a unittest.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/sysinfo/sandbox.c        | 15 +++++++++++
 drivers/sysinfo/sysinfo-uclass.c | 15 +++++++++++
 include/sysinfo.h                | 44 ++++++++++++++++++++++++++++++++
 test/dm/sysinfo.c                | 13 ++++++++++
 4 files changed, 87 insertions(+)

Comments

Simon Glass Nov. 5, 2021, 2:02 a.m. UTC | #1
On Wed, 3 Nov 2021 at 17:23, Marek Behún <kabel@kernel.org> wrote:
>
> From: Marek Behún <marek.behun@nic.cz>
>
> Add get_str_list() method to sysinfo operations.
>
> The get_str_list() method is similar to get_str(), but receives one
> additional argument, @idx, and fills in the @idx-th string from a given
> list.
>
> Add sandbox implementation together with a unittest.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>  drivers/sysinfo/sandbox.c        | 15 +++++++++++
>  drivers/sysinfo/sysinfo-uclass.c | 15 +++++++++++
>  include/sysinfo.h                | 44 ++++++++++++++++++++++++++++++++
>  test/dm/sysinfo.c                | 13 ++++++++++
>  4 files changed, 87 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Except I think it should return -NOSPC if the buffer is too small.
Marek Behún Nov. 5, 2021, 11:20 a.m. UTC | #2
On Thu, 4 Nov 2021 20:02:27 -0600
Simon Glass <sjg@chromium.org> wrote:

> On Wed, 3 Nov 2021 at 17:23, Marek Behún <kabel@kernel.org> wrote:
> >
> > From: Marek Behún <marek.behun@nic.cz>
> >
> > Add get_str_list() method to sysinfo operations.
> >
> > The get_str_list() method is similar to get_str(), but receives one
> > additional argument, @idx, and fills in the @idx-th string from a given
> > list.
> >
> > Add sandbox implementation together with a unittest.
> >
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > ---
> >  drivers/sysinfo/sandbox.c        | 15 +++++++++++
> >  drivers/sysinfo/sysinfo-uclass.c | 15 +++++++++++
> >  include/sysinfo.h                | 44 ++++++++++++++++++++++++++++++++
> >  test/dm/sysinfo.c                | 13 ++++++++++
> >  4 files changed, 87 insertions(+)
> >  
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Except I think it should return -NOSPC if the buffer is too small.

No because then you don't know how large buffer you actually need and
have to guess (or grow by a factor of two until it fits).

This way you can call with size=0, get the length, allocate buffer and
call again.

Marek
Simon Glass Nov. 5, 2021, 4:12 p.m. UTC | #3
Hi Marek,

On Fri, 5 Nov 2021 at 05:20, Marek Behún <kabel@kernel.org> wrote:
>
> On Thu, 4 Nov 2021 20:02:27 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
> > On Wed, 3 Nov 2021 at 17:23, Marek Behún <kabel@kernel.org> wrote:
> > >
> > > From: Marek Behún <marek.behun@nic.cz>
> > >
> > > Add get_str_list() method to sysinfo operations.
> > >
> > > The get_str_list() method is similar to get_str(), but receives one
> > > additional argument, @idx, and fills in the @idx-th string from a given
> > > list.
> > >
> > > Add sandbox implementation together with a unittest.
> > >
> > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > ---
> > >  drivers/sysinfo/sandbox.c        | 15 +++++++++++
> > >  drivers/sysinfo/sysinfo-uclass.c | 15 +++++++++++
> > >  include/sysinfo.h                | 44 ++++++++++++++++++++++++++++++++
> > >  test/dm/sysinfo.c                | 13 ++++++++++
> > >  4 files changed, 87 insertions(+)
> > >
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Except I think it should return -NOSPC if the buffer is too small.
>
> No because then you don't know how large buffer you actually need and
> have to guess (or grow by a factor of two until it fits).
>
> This way you can call with size=0, get the length, allocate buffer and
> call again.

OK, I completely missed that. Can you mention this approach in the comments?

"Actual length of the string" - does that mean the length that would
have been returned if there were enough space? If so, please make that
clear.

Regards,
Simon

>
> Marek
diff mbox series

Patch

diff --git a/drivers/sysinfo/sandbox.c b/drivers/sysinfo/sandbox.c
index 01c845e310..bede40b662 100644
--- a/drivers/sysinfo/sandbox.c
+++ b/drivers/sysinfo/sandbox.c
@@ -81,6 +81,20 @@  int sysinfo_sandbox_get_str(struct udevice *dev, int id, size_t size, char *val)
 	return -ENOENT;
 }
 
+int sysinfo_sandbox_get_str_list(struct udevice *dev, int id, unsigned idx,
+				 size_t size, char *val)
+{
+	if (id != STR_VACATIONSPOT)
+		return -ENOENT;
+
+	if (idx >= ARRAY_SIZE(vacation_spots))
+		return -ERANGE;
+
+	strncpy(val, vacation_spots[idx], size);
+	val[size - 1] = '\0';
+	return strlen(vacation_spots[idx]);
+}
+
 static const struct udevice_id sysinfo_sandbox_ids[] = {
 	{ .compatible = "sandbox,sysinfo-sandbox" },
 	{ /* sentinel */ }
@@ -91,6 +105,7 @@  static const struct sysinfo_ops sysinfo_sandbox_ops = {
 	.get_bool = sysinfo_sandbox_get_bool,
 	.get_int = sysinfo_sandbox_get_int,
 	.get_str = sysinfo_sandbox_get_str,
+	.get_str_list = sysinfo_sandbox_get_str_list,
 };
 
 int sysinfo_sandbox_probe(struct udevice *dev)
diff --git a/drivers/sysinfo/sysinfo-uclass.c b/drivers/sysinfo/sysinfo-uclass.c
index c5cc3cb959..71f9ad105a 100644
--- a/drivers/sysinfo/sysinfo-uclass.c
+++ b/drivers/sysinfo/sysinfo-uclass.c
@@ -92,6 +92,21 @@  int sysinfo_get_str(struct udevice *dev, int id, size_t size, char *val)
 	return ops->get_str(dev, id, size, val);
 }
 
+int sysinfo_get_str_list(struct udevice *dev, int id, unsigned idx, size_t size,
+			 char *val)
+{
+	struct sysinfo_priv *priv = dev_get_uclass_priv(dev);
+	struct sysinfo_ops *ops = sysinfo_get_ops(dev);
+
+	if (!priv->detected)
+		return -EPERM;
+
+	if (!ops->get_str_list)
+		return -ENOSYS;
+
+	return ops->get_str_list(dev, id, idx, size, val);
+}
+
 UCLASS_DRIVER(sysinfo) = {
 	.id		= UCLASS_SYSINFO,
 	.name		= "sysinfo",
diff --git a/include/sysinfo.h b/include/sysinfo.h
index 6031aec578..0d8a2d1676 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -106,6 +106,25 @@  struct sysinfo_ops {
 
 	int (*get_str)(struct udevice *dev, int id, size_t size, char *val);
 
+	/**
+	 * get_str_list() - Read a specific string data value from a string list
+	 *		    that describes hardware setup.
+	 * @dev:	The sysinfo instance to gather the data.
+	 * @id:		A unique identifier for the string list to read from.
+	 * @idx:	The index of the string in the string list.
+	 * @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: Actual length of the string excluding the terminating
+	 *	   NULL-byte if OK, -ENOENT if list with ID @id does not exist,
+	 *	   -ERANGE if @idx is invalid or -ve on error.
+	 */
+	int (*get_str_list)(struct udevice *dev, int id, unsigned idx,
+			    size_t size, char *val);
+
 	/**
 	 * get_fit_loadable - Get the name of an image to load from FIT
 	 * This function can be used to provide the image names based on runtime
@@ -180,6 +199,25 @@  int sysinfo_get_int(struct udevice *dev, int id, int *val);
  */
 int sysinfo_get_str(struct udevice *dev, int id, size_t size, char *val);
 
+/**
+ * sysinfo_get_str_list() - Read a specific string data value from a string list
+ *			    that describes hardware setup.
+ * @dev:	The sysinfo instance to gather the data.
+ * @id:		A unique identifier for the string list to read from.
+ * @idx:	The index of the string in the string list.
+ * @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: Actual length of the string excluding the terminating NULL-byte if
+ *	   OK, -ENOENT if list with ID @id does not exist, -ERANGE if @idx is
+ *	   invalid, -EPERM if called before sysinfo_detect(), else -ve on error.
+ */
+int sysinfo_get_str_list(struct udevice *dev, int id, unsigned idx, size_t size,
+			 char *val);
+
 /**
  * sysinfo_get() - Return the sysinfo device for the sysinfo in question.
  * @devp: Pointer to structure to receive the sysinfo device.
@@ -235,6 +273,12 @@  static inline int sysinfo_get_str(struct udevice *dev, int id, size_t size,
 	return -ENOSYS;
 }
 
+static inline int sysinfo_get_str_list(struct udevice *dev, int id,
+				       unsigned idx, size_t size, char *val)
+{
+	return -ENOSYS;
+}
+
 static inline int sysinfo_get(struct udevice **devp)
 {
 	return -ENOSYS;
diff --git a/test/dm/sysinfo.c b/test/dm/sysinfo.c
index 2c1bd1ce40..a6b246f2df 100644
--- a/test/dm/sysinfo.c
+++ b/test/dm/sysinfo.c
@@ -58,6 +58,19 @@  static int dm_test_sysinfo(struct unit_test_state *uts)
 				       str));
 	ut_asserteq_str(str, "Yuggoth");
 
+	ut_asserteq(6, sysinfo_get_str_list(sysinfo, STR_VACATIONSPOT, 0,
+					    sizeof(str), str));
+	ut_asserteq_str(str, "R'lyeh");
+
+	ut_asserteq(17, sysinfo_get_str_list(sysinfo, STR_VACATIONSPOT, 5, 6,
+					     str));
+	ut_asserteq_str(str, "The N");
+
+	ut_asserteq(-ENOENT, sysinfo_get_str_list(sysinfo, INT_TEST1, 0,
+						  sizeof(str), str));
+	ut_asserteq(-ERANGE, sysinfo_get_str_list(sysinfo, STR_VACATIONSPOT, 10,
+						  sizeof(str), str));
+
 	return 0;
 }