Re: [Minios-devel] [UNIKRAFT PATCH 10/11] lib/ukbus: use new list api

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Yuri,

Please find the comment inline.

Thanks & Regards
Sharan

On 12/5/18 6:23 PM, Yuri Volchkov wrote:
> Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
> ---
>   lib/ukbus/bus.c            |  9 +++------
>   lib/ukbus/include/uk/bus.h | 15 ++++-----------
>   2 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/lib/ukbus/bus.c b/lib/ukbus/bus.c
> index b8aaf22..3b3f3b3 100644
> --- a/lib/ukbus/bus.c
> +++ b/lib/ukbus/bus.c
> @@ -36,7 +36,7 @@
>   #include <uk/assert.h>
>   #include <uk/print.h>
>   
> -struct uk_bus_list uk_bus_list;
> +UK_LIST_HEAD(uk_bus_list);
>   static unsigned int bus_count;
>   
>   void _uk_bus_register(struct uk_bus *b)
> @@ -44,11 +44,8 @@ void _uk_bus_register(struct uk_bus *b)
>         UK_ASSERT(b != NULL);
>         UK_ASSERT(b->probe != NULL);
>   
> -	if (bus_count == 0)
> -               UK_TAILQ_INIT(&uk_bus_list);
> -
>         uk_pr_debug("Register bus handler: %p\n", b);
> -       UK_TAILQ_INSERT_TAIL(&uk_bus_list, b, next);
> +       uk_list_add_tail(&b->list, &uk_bus_list);
>         ++bus_count;
>   }
>   
> @@ -58,7 +55,7 @@ void _uk_bus_unregister(struct uk_bus *b)
>         UK_ASSERT(bus_count > 0);
>   
>   	uk_pr_debug("Unregister bus handler: %p\n", b);
> -       UK_TAILQ_REMOVE(&uk_bus_list, b, next);
> +       uk_list_del(&b->list);
uk_list_del has some dangling reference to the list. In this case it may 
not be a big problem. But isn't it better to use uk_list_del_init instead?
>         bus_count--;
>   }
>   
> diff --git a/lib/ukbus/include/uk/bus.h b/lib/ukbus/include/uk/bus.h
> index 0da3433..231c8d7 100644
> --- a/lib/ukbus/include/uk/bus.h
> +++ b/lib/ukbus/include/uk/bus.h
> @@ -45,24 +45,17 @@ extern "C" {
>   #endif
>   
>   struct uk_bus;
> -UK_TAILQ_HEAD(uk_bus_list, struct uk_bus);
> -extern struct uk_bus_list uk_bus_list;
> +extern struct uk_list_head uk_bus_list;
>   
>   typedef int (*uk_bus_init_func_t)(struct uk_alloc *a);
>   typedef int (*uk_bus_probe_func_t)(void);
>   
>   struct uk_bus {
> -       UK_TAILQ_ENTRY(struct uk_bus) next;
> +       struct uk_list_head list;
>         uk_bus_init_func_t init; /**< Initialize bus handler (optional) */
>         uk_bus_probe_func_t probe; /**< Probe for devices attached to the bus */
>   };
>   
> -#define UK_BUS_LIST_FOREACH(b)			\
> -       UK_TAILQ_FOREACH(b, &uk_bus_list, next)
> -
> -#define UK_BUS_LIST_FOREACH_SAFE(b, b_next)    \
> -       UK_TAILQ_FOREACH_SAFE(b, &uk_bus_list, next, b_next)
> -
>   /* Returns the number of registered buses */
>   unsigned int uk_bus_count(void);
>   
> @@ -87,7 +80,7 @@ static inline unsigned int uk_bus_init_all(struct uk_alloc *a)
>         if (uk_bus_count() == 0)
>                 return 0;
>   
> -	UK_BUS_LIST_FOREACH_SAFE(b, b_next) {
> +       uk_list_for_each_entry_safe(b, b_next, &uk_bus_list, list) {
>                 if ((status = uk_bus_init(b, a)) >= 0) {
>                         ++ret;
>                 } else {
> @@ -110,7 +103,7 @@ static inline unsigned int uk_bus_probe_all(void)
>         if (uk_bus_count() == 0)
>                 return 0;
>   
> -	UK_BUS_LIST_FOREACH(b) {
> +       uk_list_for_each_entry(b, &uk_bus_list, list) {
>                 if (uk_bus_probe(b) >= 0)
>                         ++ret;
>         }
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel