Hi Sughosh,
On Wed, 24 Jul 2024 at 00:03, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Add a couple of helper functions to detect an empty and full alist.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since rfc: None
include/alist.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
I had to hunt around to see why these are needed. It's fine to add new functions to the API, but in this case I want to make a few points.
diff --git a/include/alist.h b/include/alist.h index 6cc3161dcd..06ae137102 100644 --- a/include/alist.h +++ b/include/alist.h @@ -82,6 +82,28 @@ static inline bool alist_err(struct alist *lst) return lst->flags & ALISTF_FAIL; }
+/**
- alist_full() - Check if the alist is full
- @lst: List to check
- Return: true if full, false otherwise
- */
+static inline bool alist_full(struct alist *lst) +{
return lst->count == lst->alloc;+}
In general I see you manually modifying the members of the alist, rather than using the API to add a new item. I think it is better to use the API.
struct lmb_region rgn;
rgn.base = ... rgn.size = ... rgn.flags = ... if (!alist_add(&lmb.used, rgn. struct lmb_region)) return -ENOMEM;
or you could make a function to add a new region to a list, with base, size & flags as args.
+/**
- alist_empty() - Check if the alist is empty
- @lst: List to check
- Return: true if empty, false otherwise
- */
+static inline bool alist_empty(struct alist *lst) +{
return !lst->count && lst->alloc;+}
I would argue that this is a surprising definition of 'empty'. Why the second term? It seems to be because you want to know if it is safe to set values in item[0]. But see above for how to use the API.
/**
- alist_get_ptr() - Get the value of a pointer
-- 2.34.1
Regards, Simon