Re: [PATCH v2 08/12] sysinfo: Add support for iterating string list

Hi Marek,

On Wed, 3 Nov 2021 at 17:23, Marek Behún kabel@kernel.org wrote:

From: Marek Behún marek.behun@nic.cz

Add function sysinfo_get_str_list_max_len() to determine length of the longest string in a string list, functions sysinfo_str_list_first() and sysinfo_str_list_next() to support iteration over string list elements and macro for_each_sysinfo_str_list() to make the iteration simple to use.

Signed-off-by: Marek Behún marek.behun@nic.cz

drivers/sysinfo/sysinfo-uclass.c | 79 +++++++++++++++++++++++++ include/sysinfo.h | 99 ++++++++++++++++++++++++++++++++ test/dm/sysinfo.c | 35 +++++++++++ 3 files changed, 213 insertions(+)

diff --git a/drivers/sysinfo/sysinfo-uclass.c b/drivers/sysinfo/sysinfo-uclass.c index d945f073c5..78035a95aa 100644 --- a/drivers/sysinfo/sysinfo-uclass.c +++ b/drivers/sysinfo/sysinfo-uclass.c @@ -8,6 +8,7 @@

#include <common.h> #include <dm.h> +#include <malloc.h> #include <sysinfo.h>

struct sysinfo_priv { @@ -104,6 +105,84 @@ int sysinfo_get_str_list(struct udevice *dev, int id, unsigned idx, size_t size, return ops->get_str_list(dev, id, idx, size, val); }

+int sysinfo_get_str_list_max_len(struct udevice *dev, int id) +{

•   int maxlen = 0;
  

•   unsigned i;
  

•   /* first find out length of the longest string in the list */
  

•   for (i = 0; ; ++i) {
  

•           char dummy[1];
  

•           int len;
  

•           len = sysinfo_get_str_list(dev, id, i, 0, dummy);
  

•           if (len == -ERANGE)
  

•                   break;
  

•           else if (len < 0)
  

•                   return len;
  

•           else if (len > maxlen)
  

•                   maxlen = len;
  

•   }
  

•   return maxlen;
  

+}

+struct sysinfo_str_list_iter {

•   struct udevice *dev;
  

•   int id;
  

•   size_t size;
  

•   unsigned idx;
  

•   char value[];
  

+};

+char *sysinfo_str_list_first(struct udevice *dev, int id, void *_iter)

Better to make iter a struct sysinfo_str_list_iter, I think and require the caller to declare it:

sysinfo_str_list_iter iter; char str[80]'

p = sysinfo_str_list_first(dev, i, &str, sizeof(str), &iter); ...

Do you need the iter?

If you want to support arbitratry length, I suppose that is OK?? But I don't like allocating memory unless it is needed.

+{

•   struct sysinfo_str_list_iter *iter, **iterp = _iter;
  

•   int maxlen, res;
  

•   maxlen = sysinfo_get_str_list_max_len(dev, id);
  

•   if (maxlen < 0)
  

•           return NULL;
  

•   iter = malloc(sizeof(struct sysinfo_str_list_iter) + maxlen + 1);
  

•   if (!iter) {
  

•           printf("No memory for sysinfo string list iterator\n");
  

•           return NULL;
  

•   }
  

•   iter->dev = dev;
  

•   iter->id = id;
  

•   iter->size = maxlen + 1;
  

•   iter->idx = 0;
  

•   res = sysinfo_get_str_list(dev, id, 0, iter->size, iter->value);
  

•   if (res < 0) {
  

•           *iterp = NULL;
  

•           free(iter);
  

•           return NULL;
  

•   }
  

•   *iterp = iter;
  

•   return iter->value;
  

+}

+char *sysinfo_str_list_next(void *_iter) +{

•   struct sysinfo_str_list_iter **iterp = _iter, *iter = *iterp;
  

•   int res;
  

•   res = sysinfo_get_str_list(iter->dev, iter->id, ++iter->idx, iter->size,
  

•                              iter->value);
  

•   if (res < 0) {
  

•           *iterp = NULL;
  

•           free(iter);
  

•           return NULL;
  

•   }
  

•   return iter->value;
  

+}

UCLASS_DRIVER(sysinfo) = { .id = UCLASS_SYSINFO, .name = "sysinfo", diff --git a/include/sysinfo.h b/include/sysinfo.h index 0d8a2d1676..d32bf3e808 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -218,6 +218,61 @@ int sysinfo_get_str(struct udevice *dev, int id, size_t size, char *val); int sysinfo_get_str_list(struct udevice *dev, int id, unsigned idx, size_t size, char *val);

+/**

• • sysinfo_get_str_list_max_len() - Get length of longest string in 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.
• • Return: Length (excluding the terminating NULL-byte) of the longest string in

• •    the string list, or -ve on error.
    

• */

+int sysinfo_get_str_list_max_len(struct udevice *dev, int id);

+/**

• • sysinfo_str_list_first() - Start iterating a string list.
• • @dev: The sysinfo instance to gather the data.
• • @id: A unique identifier for the string list to read from.
• • @_iter: Pointer where iterator data will be stored.
• • Pass a reference to a void * pointer as @_iter, i.e.

• • void *iter;
    

• • first = sysinfo_str_list_first(dev, id, &iter);
    

• • The function will allocate space for the value. You need to iterate all
• • elements with sysinfo_str_list_next() for the space to be freed, or free
• • the pointer stored in @_iter, i.e.

• • void *iter;
    

• • first = sysinfo_str_list_first(dev, id, &iter);
    

• • if (first)
    

• •         free(iter);
    

• • Return: First string in the string list, or NULL on error.
• */

+char *sysinfo_str_list_first(struct udevice *dev, int id, void *_iter);

+/**

• • sysinfo_str_list_next() - Get next string in the string string list.
• • @_iter: Pointer to iterator, filled in by sysinfo_str_list_first().
• • Pass a reference to a void * pointer as @_iter, i.e.

• • void *iter;
    

• • first = sysinfo_str_list_first(dev, id, &iter);
    

• • next = sysinfo_str_list_next(&iter);
    

• • All elements must be iterated until the function returns NULL for the
• • resources allocated for the iteration to be freed, or pointer stored in
• • @_iter must be freed, i.e.:

• • void *iter;
    

• • first = sysinfo_str_list_first(dev, id, &iter);
    

• • next = sysinfo_str_list_next(&iter);
    

• • if (next)
    

• •         free(iter);
    

• • Return: Next string in the string list, NULL on end of list or NULL on error.
• */

+char *sysinfo_str_list_next(void *_iter);

/**

• sysinfo_get() - Return the sysinfo device for the sysinfo in question.
• @devp: Pointer to structure to receive the sysinfo device.

@@ -279,6 +334,22 @@ static inline int sysinfo_get_str_list(struct udevice *dev, int id, return -ENOSYS; }

+static inline int sysinfo_get_str_list_max_len(struct udevice *dev, int id) +{

•   return -ENOSYS;
  

+}

+static inline char *sysinfo_str_list_first(struct udevice *dev, int id,

•                                      void *_iter)
  

+{

•   return NULL;
  

+}

+static inline char *sysinfo_str_list_next(void *_iter) +{

•   return NULL;
  

+}

static inline int sysinfo_get(struct udevice **devp) { return -ENOSYS; @@ -291,4 +362,32 @@ static inline int sysinfo_get_fit_loadable(struct udevice *dev, int index, }

#endif

+/**

• • for_each_sysinfo_str_list - Iterate a sysinfo string list
• • @dev: The sysinfo instance to gather the data.
• • @id: A unique identifier for the string list to read from.
• • @val: String pointer for the value.
• • @iter: Pointer where iteration data are stored.
• • Important: all elements of the list must be iterated for the iterator
• • resources to be freed automatically. If you need to break from the for cycle,
• • you need to free the iterator.
• • Example:

• • char *value;
    

• • void *iter;
    

• • for_each_sysinfo_str_list(dev, MY_STR_LIST, value, iter) {
    

• •         printf("Value: %s\n", value);
    

• •         if (!strcmp(value, "needle")) {
    

• •                 free(iter);
    

• •                 break;
    

• •         }
    

• • }
    

• */

+#define for_each_sysinfo_str_list(dev, id, val, iter) \

•   for ((val) = sysinfo_str_list_first((dev), (id), &(iter));      \
  

•        (val);                                                     \
  

•        (val) = sysinfo_str_list_next(&(iter)))
  

#endif diff --git a/test/dm/sysinfo.c b/test/dm/sysinfo.c index a6b246f2df..e9b70d8e1a 100644 --- a/test/dm/sysinfo.c +++ b/test/dm/sysinfo.c @@ -75,3 +75,38 @@ static int dm_test_sysinfo(struct unit_test_state *uts) }

DM_TEST(dm_test_sysinfo, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

+static int dm_test_sysinfo_str_list_iter(struct unit_test_state *uts) +{

•   struct udevice *sysinfo;
  

•   char *value;
  

•   void *iter;
  

•   int idx;
  

•   ut_assertok(sysinfo_get(&sysinfo));
  

•   ut_assert(sysinfo);
  

•   sysinfo_detect(sysinfo);
  

•   idx = 0;
  

•   for_each_sysinfo_str_list(sysinfo, STR_VACATIONSPOT, value, iter) {
  

•           switch (idx) {
  

•           case 0:
  

•                   ut_asserteq_str(value, "R'lyeh");
  

•                   break;
  

•           case 2:
  

•                   ut_asserteq_str(value, "Plateau of Leng");
  

•                   break;
  

•           case 3:
  

•                   ut_asserteq_str(value, "Carcosa");
  

•                   break;
  

•           }
  

•           ++idx;
  

•   }
  

•   ut_assert(NULL == iter);
  

•   return 0;
  

+}

+DM_TEST(dm_test_sysinfo_str_list_iter, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

2.32.0

Regards, Simon