[PATCH v4 1/3] bloblist: add api to get blob with size

[Heinrich Schuchardt]

On 13.01.25 15:33, Raymond Mao wrote:
> Hi Heinrich,
>
> On Fri, 10 Jan 2025 at 19:04, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>> wrote:
>
>     Am 10. Januar 2025 22:56:33 MEZ schrieb Raymond Mao
>     <raymond.mao at linaro.org <mailto:raymond.mao at linaro.org>>:
>      >bloblist_find function only returns the pointer of blob data,
>      >which is fine for those self-describing data like FDT.
>      >But as a common scenario, an interface is needed to retrieve both
>      >the pointer and the size of the blob data.
>      >
>      >Add a few ut test cases for the new api.
>      >
>      >Signed-off-by: Raymond Mao <raymond.mao at linaro.org
>     <mailto:raymond.mao at linaro.org>>
>      >Reviewed-by: Simon Glass <sjg at chromium.org <mailto:sjg at chromium.org>>
>      >Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org
>     <mailto:ilias.apalodimas at linaro.org>>
>      >---
>      >Changes in v2
>      >- Rename function argument.
>      >- Add ut tests.
>      >Changes in v3
>      >- None.
>      >Changes in v4
>      >- None.
>      >
>      > common/bloblist.c      | 17 +++++++++++++++--
>      > include/bloblist.h     | 18 ++++++++++++++++++
>      > test/common/bloblist.c |  4 ++++
>      > 3 files changed, 37 insertions(+), 2 deletions(-)
>      >
>      >diff --git a/common/bloblist.c b/common/bloblist.c
>      >index 110bb9dc44..ab48a3cdb9 100644
>      >--- a/common/bloblist.c
>      >+++ b/common/bloblist.c
>      >@@ -222,14 +222,27 @@ static int bloblist_ensurerec(uint tag,
>     struct bloblist_rec **recp, int size,
>      > }
>      >
>      > void *bloblist_find(uint tag, int size)
>      >+{
>      >+      void *blob = NULL;
>      >+      int blob_size;
>      >+
>      >+      blob = bloblist_get_blob(tag, &blob_size);
>      >+
>      >+      if (size && size != blob_size)
>      >+              return NULL;
>      >+
>      >+      return blob;
>      >+}
>      >+
>      >+void *bloblist_get_blob(uint tag, int *sizep)
>      > {
>      >       struct bloblist_rec *rec;
>      >
>      >       rec = bloblist_findrec(tag);
>      >       if (!rec)
>      >               return NULL;
>      >-      if (size && size != rec->size)
>      >-              return NULL;
>      >+
>      >+      *sizep = rec->size;
>      >
>      >       return (void *)rec + rec_hdr_size(rec);
>      > }
>      >diff --git a/include/bloblist.h b/include/bloblist.h
>      >index f999391f74..52ba0ddcf8 100644
>      >--- a/include/bloblist.h
>      >+++ b/include/bloblist.h
>      >@@ -250,6 +250,24 @@ static inline void
>     *bloblist_check_magic(ulong addr)
>      >       return ptr;
>      > }
>      >
>      >+#if CONFIG_IS_ENABLED(BLOBLIST)
>
>     Why should we use an #ifdef here?
>     Why would a caller invoke the function if the configuration is disabled?
>
> Adding #ifdef here is to have the inline function so that we don't need
> to add the macro to all of its callers.

There is just one. See patch 3, where you already check the CONFIG
albeit in the wrong place.

>
>      >+/**
>      >+ * bloblist_get_blob() - Find a blob and get the size of it
>      >+ *
>      >+ * Searches the bloblist and returns the blob with the matching tag
>      >+ *
>      >+ * @tag:      Tag to search for (enum bloblist_tag_t)
>      >+ * @sizep:    Size of the blob found
>      >+ * Return: pointer to bloblist if found, or NULL if not found
>      >+ */
>      >+void *bloblist_get_blob(uint tag, int *sizep);
>
>     If tag is expected to be a value from the enum, we should not use
>     uint here.
>
> This is just following the original convention of existing bloblist
> functions which are all using uint for the tag.

@Simon: Why aren't we using enum?

Best regards

Heinrich

>
> Regards,
> Raymond
>
>
>      >+#else
>      >+static inline void *bloblist_get_blob(uint tag, int *sizep)
>      >+{
>      >+      return NULL;
>      >+}
>      >+#endif
>      >+
>      > /**
>      >  * bloblist_find() - Find a blob
>      >  *
>      >diff --git a/test/common/bloblist.c b/test/common/bloblist.c
>      >index 4bca62110a..324127596f 100644
>      >--- a/test/common/bloblist.c
>      >+++ b/test/common/bloblist.c
>      >@@ -98,10 +98,12 @@ static int bloblist_test_blob(struct
>     unit_test_state *uts)
>      >       struct bloblist_hdr *hdr;
>      >       struct bloblist_rec *rec, *rec2;
>      >       char *data;
>      >+      int size = 0;
>      >
>      >       /* At the start there should be no records */
>      >       hdr = clear_bloblist();
>      >       ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
>      >+      ut_assertnull(bloblist_get_blob(TEST_TAG, &size));
>      >       ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0));
>      >       ut_asserteq(sizeof(struct bloblist_hdr), bloblist_get_size());
>      >       ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_total_size());
>      >@@ -114,6 +116,8 @@ static int bloblist_test_blob(struct
>     unit_test_state *uts)
>      >       ut_asserteq_addr(rec + 1, data);
>      >       data = bloblist_find(TEST_TAG, TEST_SIZE);
>      >       ut_asserteq_addr(rec + 1, data);
>      >+      ut_asserteq_addr(bloblist_get_blob(TEST_TAG, &size), data);
>      >+      ut_asserteq(size, TEST_SIZE);
>      >
>      >       /* Check the data is zeroed */
>      >       ut_assertok(check_zero(data, TEST_SIZE));
>