Re: [Minios-devel] [UNIKRAFT PATCH 2/3] lib/ukboot: Modify ctor iteration to end at uk_ctortab_end

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 28.08.19 23:03, Vlad-Andrei BĂDOIU (78692) wrote:
> Hey Simon,
>
> On 28.08.2019 19:07, Simon Kuenzer wrote:
> > On 27.08.19 19:28, Vlad-Andrei BĂDOIU (78692) wrote:
> > > The iteration thourgh the ctortab array of constructors now ends at
> > > uk_ctortab_end.
> > >
> > > Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
> > > ---
> > >    include/uk/ctors.h | 1 +
> > >    lib/ukboot/boot.c  | 2 +-
> > >    2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/uk/ctors.h b/include/uk/ctors.h
> > > index a3432e2b..4223b338 100644
> > > --- a/include/uk/ctors.h
> > > +++ b/include/uk/ctors.h
> > > @@ -45,6 +45,7 @@ extern "C" {
> > >      typedef void (*uk_ctor_func_t)(void);
> > >    extern const uk_ctor_func_t uk_ctortab[];
> > > +extern const uk_ctor_func_t uk_ctortab_end[];
> > I would declare this as
> >      extern const uk_ctor_func_t uk_ctortab_end;
> >
> > Then you can re-use the already existing foreach macro which is
> > currently within <uk/plat/ctors.h>
> > >      /*
> > >     * Register a constructor function that is
> > > diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
> > > index 9738a912..935123c7 100644
> > > --- a/lib/ukboot/boot.c
> > > +++ b/lib/ukboot/boot.c
> > > @@ -182,7 +182,7 @@ void ukplat_entry(int argc, char *argv[])
> > >    #endif
> > >          uk_pr_info("Unikraft constructors table at %p\n", uk_ctortab);
> > > -    for (cfn = uk_ctortab; *cfn != NULL; ++cfn) {
> > > +    for (cfn = uk_ctortab; cfn != &uk_ctortab_end; ++cfn) {
> > Logically, the '&' is de-referencing the end address of the list. It
> > is too much in this line. However, I would prefer using the existing
> > foreach macro instead.
> I agree. I initially thought about using the foreach macro but passed
> over the idea in order to keep the amount of changes to the minimum.
> I'll add the proposed changes in the v2 of this patch.
Thanks a lot. I agree that usually we should keep the changes minimal, 
you are completely right. In this special case, I think that the 
restructuring is getting us a cleaner API while we still do not bloat 
the changes too much. This is why I think it is worth pushing for it.
> > >            uk_pr_debug("Call constructor %p\n", *cfn);
> > >            (*cfn)();
> > >        }
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel