Hi Heinrich, On Fri, 10 Jan 2025 at 19:04, Heinrich Schuchardt <[email protected]> wrote:
> Am 10. Januar 2025 22:56:33 MEZ schrieb Raymond Mao < > [email protected]>: > >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 <[email protected]> > >Reviewed-by: Simon Glass <[email protected]> > >Reviewed-by: Ilias Apalodimas <[email protected]> > >--- > >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)); > >
