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

[Raymond Mao]

Hi Heinrich,

On Fri, 10 Jan 2025 at 19:04, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> Am 10. Januar 2025 22:56:33 MEZ schrieb Raymond Mao <
> 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>
> >Reviewed-by: Simon Glass <sjg at chromium.org>
> >Reviewed-by: Ilias Apalodimas <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.


> >+/**
> >+ * 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.

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));
>
>