From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 939D0C433F5 for ; Fri, 5 Nov 2021 02:04:17 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C1FB760E8C for ; Fri, 5 Nov 2021 02:04:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C1FB760E8C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3795783743; Fri, 5 Nov 2021 03:03:49 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="G18irROR"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D7EA783686; Fri, 5 Nov 2021 03:03:07 +0100 (CET) Received: from mail-ot1-x329.google.com (mail-ot1-x329.google.com [IPv6:2607:f8b0:4864:20::329]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id D9F428370B for ; Fri, 5 Nov 2021 03:02:45 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ot1-x329.google.com with SMTP id s19-20020a056830125300b0055ad9673606so9719374otp.0 for ; Thu, 04 Nov 2021 19:02:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=OiaOwDayDXx1gt/ceo5Y5m9Kln1DVXmChb/fAL/z1Ks=; b=G18irRORx0cROuk0qu+J7gpgSn84Ko+rpuGlLOAgWFya9W0zzDEMbIwfXPFPZl9+Lm Ha52pzxW4Gl78bvnH4jX7fs2tpA0XvaXMFBB2TB+L2EJd1CA9BN9Z6prDQfrB9p9vK3Y 8UP7WTR0rKxd6sNX3j/sBOHvhhzCaQsjw7KRE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=OiaOwDayDXx1gt/ceo5Y5m9Kln1DVXmChb/fAL/z1Ks=; b=wpDz5bT3nW9E124wTaMUQfaaTHhxHMh7A5Fp1KJCvfhIZ3zXAWXAKIRY+ZSQs6P4Xo cdg8DLFvGpOW2vABMxyx+6k1gSWtB+JazT5wjON71I2UztNt+kMhr41uT65uU4ak2p/u HDEZ8AIeOUlIEQ93fkT0zcSmlV2KMBaOXJc3AY/N/Y4C2/WIhDhr36cpae3ZKCt4b+/t /Rv8VTNa8qeBTvunEieFweQM8ab+i2dHHQJbYCBjvSiwF4bI0Ih/ETeZCnEWY+z1nvys z4MQUIZhByVuizlw0HxYwLdnpbOz0eIwnFR9VmEUF3f8DpCBDsfNtv2kH42ayIOk7RCD WLsw== X-Gm-Message-State: AOAM533UzfXRjLMest2kdSbt59pldvlTc+qA9DssRImy97rUr2/YJYGU YtCx0gGlQjOiMp2P0FOYDd+YLPRDEAzEqO+f/7HTBJ1CY+M= X-Google-Smtp-Source: ABdhPJy4dgHu83Hf1JH4yp+c+Wsk/xRowdag/hwjfUREa7/9Ov6jNCc+qrVMEdGgW7ZRvPXONaviPf9JlBkQm0kVHYY= X-Received: by 2002:a05:6830:90:: with SMTP id a16mr5121865oto.97.1636077764261; Thu, 04 Nov 2021 19:02:44 -0700 (PDT) MIME-Version: 1.0 References: <20211103232332.2737-1-kabel@kernel.org> <20211103232332.2737-9-kabel@kernel.org> In-Reply-To: <20211103232332.2737-9-kabel@kernel.org> From: Simon Glass Date: Thu, 4 Nov 2021 20:02:29 -0600 Message-ID: Subject: Re: [PATCH v2 08/12] sysinfo: Add support for iterating string list To: =?UTF-8?B?TWFyZWsgQmVow7pu?= Cc: =?UTF-8?Q?Pali_Roh=C3=A1r?= , u-boot@lists.denx.de, =?UTF-8?B?TWFyZWsgQmVow7pu?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Marek, On Wed, 3 Nov 2021 at 17:23, Marek Beh=C3=BAn wrote: > > From: Marek Beh=C3=BAn > > 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=C3=BAn > --- > 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-u= class.c > index d945f073c5..78035a95aa 100644 > --- a/drivers/sysinfo/sysinfo-uclass.c > +++ b/drivers/sysinfo/sysinfo-uclass.c > @@ -8,6 +8,7 @@ > > #include > #include > +#include > #include > > 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 =3D 0; > + unsigned i; > + > + /* first find out length of the longest string in the list */ > + for (i =3D 0; ; ++i) { > + char dummy[1]; > + int len; > + > + len =3D sysinfo_get_str_list(dev, id, i, 0, dummy); > + if (len =3D=3D -ERANGE) > + break; > + else if (len < 0) > + return len; > + else if (len > maxlen) > + maxlen =3D 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 =3D 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 =3D _iter; > + int maxlen, res; > + > + maxlen =3D sysinfo_get_str_list_max_len(dev, id); > + if (maxlen < 0) > + return NULL; > + > + iter =3D malloc(sizeof(struct sysinfo_str_list_iter) + maxlen + 1= ); > + if (!iter) { > + printf("No memory for sysinfo string list iterator\n"); > + return NULL; > + } > + > + iter->dev =3D dev; > + iter->id =3D id; > + iter->size =3D maxlen + 1; > + iter->idx =3D 0; > + > + res =3D sysinfo_get_str_list(dev, id, 0, iter->size, iter->value)= ; > + if (res < 0) { > + *iterp =3D NULL; > + free(iter); > + return NULL; > + } > + > + *iterp =3D iter; > + > + return iter->value; > +} > + > +char *sysinfo_str_list_next(void *_iter) > +{ > + struct sysinfo_str_list_iter **iterp =3D _iter, *iter =3D *iterp; > + int res; > + > + res =3D sysinfo_get_str_list(iter->dev, iter->id, ++iter->idx, it= er->size, > + iter->value); > + if (res < 0) { > + *iterp =3D NULL; > + free(iter); > + return NULL; > + } > + > + return iter->value; > +} > + > UCLASS_DRIVER(sysinfo) =3D { > .id =3D UCLASS_SYSINFO, > .name =3D "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, siz= e_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 st= ring > + * list that describes hardware setup. > + * @dev: The sysinfo instance to gather the data. > + * @id: A unique identifier for the string list to read f= rom. > + * > + * Return: Length (excluding the terminating NULL-byte) of the longest s= tring 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 f= rom. > + * @_iter: Pointer where iterator data will be stored. > + * > + * Pass a reference to a void * pointer as @_iter, i.e. > + * void *iter; > + * first =3D sysinfo_str_list_first(dev, id, &iter); > + * > + * The function will allocate space for the value. You need to iterate a= ll > + * elements with sysinfo_str_list_next() for the space to be freed, or f= ree > + * the pointer stored in @_iter, i.e. > + * void *iter; > + * first =3D 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 =3D sysinfo_str_list_first(dev, id, &iter); > + * next =3D 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 =3D sysinfo_str_list_first(dev, id, &iter); > + * next =3D sysinfo_str_list_next(&iter); > + * if (next) > + * free(iter); > + * > + * Return: Next string in the string list, NULL on end of list or NULL o= n 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 udevic= e *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 ud= evice *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 f= rom. > + * @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 fo= r 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) =3D sysinfo_str_list_first((dev), (id), &(iter)); = \ > + (val); \ > + (val) =3D 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 =3D 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 =3D=3D iter); > + > + return 0; > +} > + > +DM_TEST(dm_test_sysinfo_str_list_iter, UT_TESTF_SCAN_PDATA | UT_TESTF_SC= AN_FDT); > -- > 2.32.0 > Regards, Simon