[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 06/01/2018 04:12 PM, Yuri Volchkov wrote:
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?
I am ok with current version of the patch, which does not have second
macro at all. It was Sharan's complain :).

If you want to reissue this patch, then I would prefer
__constructor_prio() over __constructorp(). That's all.

Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:


I am fine with it. If we need to extend on the constructor interface we might it when necessary.

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?


I wanted to clarify if we may run into issue with the order of the constructor execution. A glance in the gcc documentation, does not quite mention anything on the order of the execution of constructors without priority but it is more logical for those constructor to execute at the end.

* https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Function-Attributes.html

+
    #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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.