Re: [Minios-devel] [UNIKRAFT PATCH v3 5/7] include/essentials: Provide __constructor macro

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 01.06.2018 13:30, Yuri Volchkov wrote:
> Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:
>
> > On 01.06.2018 11:31, Sharan Santhanam wrote:
> > > On 05/30/2018 04:18 PM, Simon Kuenzer wrote:
> > > > On 23.05.2018 15:00, Sharan Santhanam wrote:
> > > > > Hello Simon,
> > > > >
> > > > >
> > > > > Please find my comments in line.
> > > > >
> > > > >
> > > > > On 05/22/2018 02:20 PM, Simon Kuenzer wrote:
> > > > > > Provide a constructor attribure macro for marking a
> > > > > > function symbol as constructor. The linker/compiler
> > > > > > is going to populate a function pointer of it to
> > > > > > the init_array section of the binary.
> > > > > >
> > > > > > Signed-off-by: Simon Kuenzer<simon.kuenzer@xxxxxxxxx>
> > > > > > ---
> > > > > >    include/uk/essentials.h | 13 +++++++++++++
> > > > > >    1 file changed, 13 insertions(+)
> > > > > >
> > > > > > diff --git a/include/uk/essentials.h b/include/uk/essentials.h
> > > > > > index f6cc6ea..3d1b705 100644
> > > > > > --- a/include/uk/essentials.h
> > > > > > +++ b/include/uk/essentials.h
> > > > > > @@ -73,6 +73,19 @@ extern "C" {
> > > > > >    #ifndef __align
> > > > > >    #define __align(bytes)         __attribute__((aligned(bytes)))
> > > > > >    #endif
> > > > > > +
> > > > > > +/**
> > > > > > + * Mark a function as constructor
> > > > > > + * The compiler/linker will populate a function pointer
> > > > > > + * (sorted by priority) to the init_array section
> > > > > > + *
> > > > > > + * @param lvl
> > > > > > + *   Priority level (101 (earliest)...onwards (latest))
> > > > > > + */
> > > > > > +#ifndef __constructor
> > > > > > +#define __constructor(lvl) __attribute__ ((constructor (lvl)))
> > > > > > +#endif
> > > > > The lvl parameter is an optional parameter. We are forcing the user
> > > > > to use constructor with priority as the default case. Is there any
> > > > > reason behind it?
> > > > I wanted to expose the richer functionality with the macro and making
> > > > the programmer aware that there is a order in which constructors are
> > > > called. The user can still use `__attribute__((constructor))` but this
> > > > one would have lower priority than the ones with a level parameter.
> > > >
> > > > I am fine if you say a macro for both should be there. Is this the case?
> > > I agree in exposing the lvl parameter, but if the user of the library is
> > > going to directly "__attribute__((constructor))" we may have
> > > difficulties in enforcing the order of the constructor, if that is
> > > necessary. So I do not like that part as a part of the application
> > > porting to Unikraft might require the user to know the order of the
> > > constructor across the different libraries.
> Actually, if an application needs __constructor, it is already far
> beyond normal api. This means one already have to think about lower
> level things. In my opinion thinking about levels is just another one
> tiny step down, which does not make that big difference.
>
> > If the user is using __attribute__((constructor)) directly (without
> > lvl), such functions will be called after the prioritized ones.
> > Constructors with a priority level always called before the non-leveld ones.
> >
> > Would this avoid difficulties?
> >
> > /**
> >    * Mark a function as constructor
> >    * The compiler/linker will populate a function pointer
> >    * to the init_array section
> >    */
> > #ifndef __constructor
> > #define __constructor __attribute__ ((constructor))
> > #endif
> >
> > /**
> >    * Mark a function as constructor with priority
> >    * The compiler/linker will populate a function pointer
> >    * (sorted by priority) to the init_array section
> >    * Prioritized constructors are called before
> >    * non-prioritized ones
> >    *
> >    * @param lvl
> >    *   Priority level (101 (earliest)...onwards (latest))
> >    */
> > #ifndef __constructorp
> > #define __constructorp(lvl) __attribute__ ((constructor (lvl)))
> > #endif
> I am fine with 2 macros, but I failed to figure out what 'p' in the end
> stands for :). Maybe name it __constructor_lvl()?
;-) Okay, I'll go for __constructor_prio()
You want a resubmission for this or shall I go ahead?

> > > > > > +
> > > > > >    #else
> > > > > >    /* TO BE DEFINED */
> > > > > >    #endif /* __GNUC__ */
> > > > > Thanks & Regards
> > > > > Sharan Santhanam
> > > Thanks & Regards
> > > Sharan
> > _______________________________________________
> > Minios-devel mailing list
> > Minios-devel@xxxxxxxxxxxxxxxxxxxx
> > https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel