validation: new functions to register hooks

Message ID 1435146697-2520-1-git-send-email-christophe.milard@linaro.org
State New
Headers show

Commit Message

Christophe Milard June 24, 2015, 11:51 a.m.
Two new functions are introduced in odp_cunit_common.c:
odp_cunit_register_global_init() is to be used to register test
executable init function (overloading the default ODP init performed).
odp_cunit_register_global_term() is to be used to register test
executable terminason function (overloading the default ODP term).
These two functions should be used instead of overloading the default weak
symbol. Overloading the weak symbol still works, but is planned for
removal.
The usage of these register functions enables the init/term functions to
be part of the library for the tested module.

Signed-off-by: Christophe Milard <christophe.milard@linaro.org>
---
 test/validation/common/odp_cunit_common.c | 37 +++++++++++++++++++++++++++++--
 test/validation/common/odp_cunit_common.h | 21 +++++++++++++-----
 2 files changed, 51 insertions(+), 7 deletions(-)

Comments

Stuart Haslam June 29, 2015, 10:22 a.m. | #1
On Wed, Jun 24, 2015 at 01:51:37PM +0200, Christophe Milard wrote:
> Two new functions are introduced in odp_cunit_common.c:
> odp_cunit_register_global_init() is to be used to register test
> executable init function (overloading the default ODP init performed).
> odp_cunit_register_global_term() is to be used to register test
> executable terminason function (overloading the default ODP term).
> These two functions should be used instead of overloading the default weak
> symbol. Overloading the weak symbol still works, but is planned for
> removal.
> The usage of these register functions enables the init/term functions to
> be part of the library for the tested module.
> 
> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>
> ---
>  test/validation/common/odp_cunit_common.c | 37 +++++++++++++++++++++++++++++--
>  test/validation/common/odp_cunit_common.h | 21 +++++++++++++-----
>  2 files changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/test/validation/common/odp_cunit_common.c b/test/validation/common/odp_cunit_common.c
> index 2b83ae6..67938f2 100644
> --- a/test/validation/common/odp_cunit_common.c
> +++ b/test/validation/common/odp_cunit_common.c
> @@ -11,6 +11,15 @@
>  /* Globals */
>  static odph_linux_pthread_t thread_tbl[MAX_WORKERS];
>  
> +/*
> + * global init/term functions which may be registered
> + * defaults to functions performing odp init/term.
> + */
> +static struct {
> +	int (*global_init_ptr)(void);
> +	int (*global_term_ptr)(void);
> +} global_init_term = {tests_global_init, tests_global_term};
> +
>  /** create test thread */
>  int odp_cunit_thread_create(void *func_ptr(void *), pthrd_arg *arg)
>  {
> @@ -62,6 +71,26 @@ ODP_WEAK_SYMBOL int tests_global_term(void)
>  	return 0;
>  }
>  
> +/*
> + * register tests_global_init and tests_global_term functions.
> + * If some of these functions are not registered, the defaults functions
> + * (tests_global_init() and tests_global_term()) defined above are used.
> + * One should use these register functions when defining these hooks.
> + * (overloading the weak symbol above is obsolete and will be removed in
> + * the future).

Is there any reason (other than time) not to just get on and remove
these now? it's best to avoid future promises like this if possible as
it looks bad when they're still there in 2 years. I'm OK with it staying
like this until the current restructuring series are merged, but the
tidy up should be done asap afterwards.

> + * Note that passing NULL as function pointer is valid and will simply
> + * prevent the default (odp init/term) to be done.
> + */
> +void odp_cunit_register_global_init(int (*func_init_ptr)(void))
> +{
> +	global_init_term.global_init_ptr = func_init_ptr;
> +}
> +
> +void odp_cunit_register_global_term(int (*func_term_ptr)(void))
> +{
> +	global_init_term.global_term_ptr = func_term_ptr;
> +}
> +
>  int odp_cunit_run(CU_SuiteInfo testsuites[])
>  {
>  	int ret;
> @@ -69,7 +98,9 @@ int odp_cunit_run(CU_SuiteInfo testsuites[])
>  	printf("\tODP API version: %s\n", odp_version_api_str());
>  	printf("\tODP implementation version: %s\n", odp_version_impl_str());
>  
> -	if (0 != tests_global_init())
> +	/* call test executable init hook, if any */
> +	if (global_init_term.global_init_ptr &&
> +	    ((*global_init_term.global_init_ptr)() != 0))
>  		return -1;
>  
>  	CU_set_error_action(CUEA_ABORT);
> @@ -83,7 +114,9 @@ int odp_cunit_run(CU_SuiteInfo testsuites[])
>  
>  	CU_cleanup_registry();
>  
> -	if (0 != tests_global_term())
> +	/* call test executable terminason hook, if any */
> +	if (global_init_term.global_term_ptr &&
> +	    ((*global_init_term.global_term_ptr)() != 0))
>  		return -1;
>  
>  	return (ret) ? -1 : 0;
> diff --git a/test/validation/common/odp_cunit_common.h b/test/validation/common/odp_cunit_common.h
> index 3b4d9fb..7d02a01 100644
> --- a/test/validation/common/odp_cunit_common.h
> +++ b/test/validation/common/odp_cunit_common.h
> @@ -43,13 +43,20 @@ typedef struct {
>  /** create thread fro start_routine function */
>  extern int odp_cunit_thread_create(void *func_ptr(void *), pthrd_arg *arg);
>  extern int odp_cunit_thread_exit(pthrd_arg *);
> +
>  /**
> - * Global tests initialization.
> + * Global tests initialization/terminason.

termination

>   *
> - * Initialize global resources needed by all testsuites. Default weak definition
> - * do nothing. Test application can override it by defining a strong version.
> - * The function is called by the common main() just after ODP global/local
> - * initialization.
> + * Initialize global resources needed by the test executable. Default
> + * definition does ODP init / term (both global and local).
> + * Test executables can override it by calling one of the
> + * register function below (or by defining a strong version,
> + * but this is depreciated).

deprecated

> + * The functions are called at the very beginning
> + * end very end of the test execution.

and

Also line wrapping isn't consistent.

> + * Passing NULL to odp_cunit_register_global_init() and/or
> + * odp_cunit_register_global_term() is legal and will simply prevent the
> + * default (ODP init/term) to be done.
>   *
>   * @note: This function is a workaround for Crypto test and other applications
>   *        should try not to use it, because it will complicate migration to a

Shouldn't this @note be removed now?
Christophe Milard June 29, 2015, 11:03 a.m. | #2
On 29 June 2015 at 12:22, Stuart Haslam <stuart.haslam@linaro.org> wrote:

> On Wed, Jun 24, 2015 at 01:51:37PM +0200, Christophe Milard wrote:
> > Two new functions are introduced in odp_cunit_common.c:
> > odp_cunit_register_global_init() is to be used to register test
> > executable init function (overloading the default ODP init performed).
> > odp_cunit_register_global_term() is to be used to register test
> > executable terminason function (overloading the default ODP term).
> > These two functions should be used instead of overloading the default
> weak
> > symbol. Overloading the weak symbol still works, but is planned for
> > removal.
> > The usage of these register functions enables the init/term functions to
> > be part of the library for the tested module.
> >
> > Signed-off-by: Christophe Milard <christophe.milard@linaro.org>
> > ---
> >  test/validation/common/odp_cunit_common.c | 37
> +++++++++++++++++++++++++++++--
> >  test/validation/common/odp_cunit_common.h | 21 +++++++++++++-----
> >  2 files changed, 51 insertions(+), 7 deletions(-)
> >
> > diff --git a/test/validation/common/odp_cunit_common.c
> b/test/validation/common/odp_cunit_common.c
> > index 2b83ae6..67938f2 100644
> > --- a/test/validation/common/odp_cunit_common.c
> > +++ b/test/validation/common/odp_cunit_common.c
> > @@ -11,6 +11,15 @@
> >  /* Globals */
> >  static odph_linux_pthread_t thread_tbl[MAX_WORKERS];
> >
> > +/*
> > + * global init/term functions which may be registered
> > + * defaults to functions performing odp init/term.
> > + */
> > +static struct {
> > +     int (*global_init_ptr)(void);
> > +     int (*global_term_ptr)(void);
> > +} global_init_term = {tests_global_init, tests_global_term};
> > +
> >  /** create test thread */
> >  int odp_cunit_thread_create(void *func_ptr(void *), pthrd_arg *arg)
> >  {
> > @@ -62,6 +71,26 @@ ODP_WEAK_SYMBOL int tests_global_term(void)
> >       return 0;
> >  }
> >
> > +/*
> > + * register tests_global_init and tests_global_term functions.
> > + * If some of these functions are not registered, the defaults functions
> > + * (tests_global_init() and tests_global_term()) defined above are used.
> > + * One should use these register functions when defining these hooks.
> > + * (overloading the weak symbol above is obsolete and will be removed in
> > + * the future).
>
> Is there any reason (other than time) not to just get on and remove
> these now? it's best to avoid future promises like this if possible as
> it looks bad when they're still there in 2 years. I'm OK with it staying
> like this until the current restructuring series are merged, but the
> tidy up should be done asap afterwards.
>

These weak defines are used by the crypto module. I did not change it there
because it was a single executable, and the focus was moving to the
platform side. It was looking worse in "init" where we are having 3
executables redifining these functions. But I definitively think it should
be consistent, i.e. when this gets in, I'll send a patch for crypto. The
module init/term function will be called:
<Module>_init[_*] and <Module>_term[_*]
Where the possible suffix (_*) identifies the test executable if needed.
These function will be part of the module lib.

I will then remove the weak definition.


> > + * Note that passing NULL as function pointer is valid and will simply
> > + * prevent the default (odp init/term) to be done.
> > + */
> > +void odp_cunit_register_global_init(int (*func_init_ptr)(void))
> > +{
> > +     global_init_term.global_init_ptr = func_init_ptr;
> > +}
> > +
> > +void odp_cunit_register_global_term(int (*func_term_ptr)(void))
> > +{
> > +     global_init_term.global_term_ptr = func_term_ptr;
> > +}
> > +
> >  int odp_cunit_run(CU_SuiteInfo testsuites[])
> >  {
> >       int ret;
> > @@ -69,7 +98,9 @@ int odp_cunit_run(CU_SuiteInfo testsuites[])
> >       printf("\tODP API version: %s\n", odp_version_api_str());
> >       printf("\tODP implementation version: %s\n",
> odp_version_impl_str());
> >
> > -     if (0 != tests_global_init())
> > +     /* call test executable init hook, if any */
> > +     if (global_init_term.global_init_ptr &&
> > +         ((*global_init_term.global_init_ptr)() != 0))
> >               return -1;
> >
> >       CU_set_error_action(CUEA_ABORT);
> > @@ -83,7 +114,9 @@ int odp_cunit_run(CU_SuiteInfo testsuites[])
> >
> >       CU_cleanup_registry();
> >
> > -     if (0 != tests_global_term())
> > +     /* call test executable terminason hook, if any */
> > +     if (global_init_term.global_term_ptr &&
> > +         ((*global_init_term.global_term_ptr)() != 0))
> >               return -1;
> >
> >       return (ret) ? -1 : 0;
> > diff --git a/test/validation/common/odp_cunit_common.h
> b/test/validation/common/odp_cunit_common.h
> > index 3b4d9fb..7d02a01 100644
> > --- a/test/validation/common/odp_cunit_common.h
> > +++ b/test/validation/common/odp_cunit_common.h
> > @@ -43,13 +43,20 @@ typedef struct {
> >  /** create thread fro start_routine function */
> >  extern int odp_cunit_thread_create(void *func_ptr(void *), pthrd_arg
> *arg);
> >  extern int odp_cunit_thread_exit(pthrd_arg *);
> > +
> >  /**
> > - * Global tests initialization.
> > + * Global tests initialization/terminason.
>
> termination
>

Fixed in V2


> >   *
> > - * Initialize global resources needed by all testsuites. Default weak
> definition
> > - * do nothing. Test application can override it by defining a strong
> version.
> > - * The function is called by the common main() just after ODP
> global/local
> > - * initialization.
> > + * Initialize global resources needed by the test executable. Default
> > + * definition does ODP init / term (both global and local).
> > + * Test executables can override it by calling one of the
> > + * register function below (or by defining a strong version,
> > + * but this is depreciated).
>
> deprecated
>

Fixed in V2


> > + * The functions are called at the very beginning
> > + * end very end of the test execution.
>
> and
>

Fixed in v2


> Also line wrapping isn't consistent.
>

Fixed in v2

>
> > + * Passing NULL to odp_cunit_register_global_init() and/or
> > + * odp_cunit_register_global_term() is legal and will simply prevent the
> > + * default (ODP init/term) to be done.
> >   *
> >   * @note: This function is a workaround for Crypto test and other
> applications
> >   *        should try not to use it, because it will complicate
> migration to a
>
> Shouldn't this @note be removed now?
>

Was not sure either. I don't really like the usage of those... But ok:
removed in v2 !


> --
> Stuart.
>

Thanks
Christophe

Patch

diff --git a/test/validation/common/odp_cunit_common.c b/test/validation/common/odp_cunit_common.c
index 2b83ae6..67938f2 100644
--- a/test/validation/common/odp_cunit_common.c
+++ b/test/validation/common/odp_cunit_common.c
@@ -11,6 +11,15 @@ 
 /* Globals */
 static odph_linux_pthread_t thread_tbl[MAX_WORKERS];
 
+/*
+ * global init/term functions which may be registered
+ * defaults to functions performing odp init/term.
+ */
+static struct {
+	int (*global_init_ptr)(void);
+	int (*global_term_ptr)(void);
+} global_init_term = {tests_global_init, tests_global_term};
+
 /** create test thread */
 int odp_cunit_thread_create(void *func_ptr(void *), pthrd_arg *arg)
 {
@@ -62,6 +71,26 @@  ODP_WEAK_SYMBOL int tests_global_term(void)
 	return 0;
 }
 
+/*
+ * register tests_global_init and tests_global_term functions.
+ * If some of these functions are not registered, the defaults functions
+ * (tests_global_init() and tests_global_term()) defined above are used.
+ * One should use these register functions when defining these hooks.
+ * (overloading the weak symbol above is obsolete and will be removed in
+ * the future).
+ * Note that passing NULL as function pointer is valid and will simply
+ * prevent the default (odp init/term) to be done.
+ */
+void odp_cunit_register_global_init(int (*func_init_ptr)(void))
+{
+	global_init_term.global_init_ptr = func_init_ptr;
+}
+
+void odp_cunit_register_global_term(int (*func_term_ptr)(void))
+{
+	global_init_term.global_term_ptr = func_term_ptr;
+}
+
 int odp_cunit_run(CU_SuiteInfo testsuites[])
 {
 	int ret;
@@ -69,7 +98,9 @@  int odp_cunit_run(CU_SuiteInfo testsuites[])
 	printf("\tODP API version: %s\n", odp_version_api_str());
 	printf("\tODP implementation version: %s\n", odp_version_impl_str());
 
-	if (0 != tests_global_init())
+	/* call test executable init hook, if any */
+	if (global_init_term.global_init_ptr &&
+	    ((*global_init_term.global_init_ptr)() != 0))
 		return -1;
 
 	CU_set_error_action(CUEA_ABORT);
@@ -83,7 +114,9 @@  int odp_cunit_run(CU_SuiteInfo testsuites[])
 
 	CU_cleanup_registry();
 
-	if (0 != tests_global_term())
+	/* call test executable terminason hook, if any */
+	if (global_init_term.global_term_ptr &&
+	    ((*global_init_term.global_term_ptr)() != 0))
 		return -1;
 
 	return (ret) ? -1 : 0;
diff --git a/test/validation/common/odp_cunit_common.h b/test/validation/common/odp_cunit_common.h
index 3b4d9fb..7d02a01 100644
--- a/test/validation/common/odp_cunit_common.h
+++ b/test/validation/common/odp_cunit_common.h
@@ -43,13 +43,20 @@  typedef struct {
 /** create thread fro start_routine function */
 extern int odp_cunit_thread_create(void *func_ptr(void *), pthrd_arg *arg);
 extern int odp_cunit_thread_exit(pthrd_arg *);
+
 /**
- * Global tests initialization.
+ * Global tests initialization/terminason.
  *
- * Initialize global resources needed by all testsuites. Default weak definition
- * do nothing. Test application can override it by defining a strong version.
- * The function is called by the common main() just after ODP global/local
- * initialization.
+ * Initialize global resources needed by the test executable. Default
+ * definition does ODP init / term (both global and local).
+ * Test executables can override it by calling one of the
+ * register function below (or by defining a strong version,
+ * but this is depreciated).
+ * The functions are called at the very beginning
+ * end very end of the test execution.
+ * Passing NULL to odp_cunit_register_global_init() and/or
+ * odp_cunit_register_global_term() is legal and will simply prevent the
+ * default (ODP init/term) to be done.
  *
  * @note: This function is a workaround for Crypto test and other applications
  *        should try not to use it, because it will complicate migration to a
@@ -60,4 +67,8 @@  extern int tests_global_init(void);
 
 extern int tests_global_term(void);
 
+void odp_cunit_register_global_init(int (*func_init_ptr)(void));
+
+void odp_cunit_register_global_term(int (*func_term_ptr)(void));
+
 #endif /* ODP_CUNICT_COMMON_H */