On 5 December 2014 at 13:10, Stuart Haslam <[email protected]> wrote:
> On Thu, Dec 04, 2014 at 05:50:59PM +0000, Mike Holmes wrote: > > Signed-off-by: Mike Holmes <[email protected]> > > --- > > test/validation/.gitignore | 1 + > > test/validation/Makefile.am | 4 ++- > > test/validation/odp_system.c | 73 > ++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 77 insertions(+), 1 deletion(-) > > create mode 100644 test/validation/odp_system.c > > > > I do wonder whether some of these APIs should exist at all. but anyway > The push is to cover the current repo with tests The 1.0 Delta <https://docs.google.com/a/linaro.org/document/d/1BRVyW8IIVMTq4nhB_vUz5y-te6TEdu5g1XgolujjY6c/edit> doc has a note in there now as a question for Petri on keeping these public. > I've made some comments below. > > > diff --git a/test/validation/.gitignore b/test/validation/.gitignore > > index 37e2594..e3b86f6 100644 > > --- a/test/validation/.gitignore > > +++ b/test/validation/.gitignore > > @@ -4,3 +4,4 @@ odp_init > > odp_queue > > odp_crypto > > odp_shm > > +odp_system > > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am > > index 8547085..f85166a 100644 > > --- a/test/validation/Makefile.am > > +++ b/test/validation/Makefile.am > > @@ -6,13 +6,14 @@ AM_LDFLAGS += -static > > if ODP_CUNIT_ENABLED > > TESTS = ${bin_PROGRAMS} > > check_PROGRAMS = ${bin_PROGRAMS} > > -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm > > +bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_system > > odp_init_LDFLAGS = $(AM_LDFLAGS) > > odp_queue_LDFLAGS = $(AM_LDFLAGS) > > odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto > > odp_crypto_LDFLAGS = $(AM_LDFLAGS) > > odp_shm_CFLAGS = $(AM_CFLAGS) > > odp_shm_LDFLAGS = $(AM_LDFLAGS) > > +odp_system_LDFLAGS = $(AM_LDFLAGS) > > endif > > > > dist_odp_init_SOURCES = odp_init.c > > @@ -22,3 +23,4 @@ dist_odp_crypto_SOURCES = > crypto/odp_crypto_test_async_inp.c \ > > crypto/odp_crypto_test_rng.c \ > > odp_crypto.c common/odp_cunit_common.c > > dist_odp_shm_SOURCES = odp_shm.c common/odp_cunit_common.c > > +dist_odp_system_SOURCES = odp_system.c common/odp_cunit_common.c > > diff --git a/test/validation/odp_system.c b/test/validation/odp_system.c > > new file mode 100644 > > index 0000000..193b8f3 > > --- /dev/null > > +++ b/test/validation/odp_system.c > > @@ -0,0 +1,73 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +#include "odp.h" > > +#include "odp_cunit_common.h" > > + > > +static void test_odp_sys_core_count(void) > > +{ > > + int cores; > > + > > + cores = odp_sys_core_count(); > > + CU_ASSERT(0 < cores) > > missing ; here and on all other asserts. I guess it works like this but > my brain thinks it's a syntax error, besides we should be consistent and > it seems other tests add the ; > Will fix > > > +} > > + > > +static void test_odp_sys_cache_line_size(void) > > +{ > > + uint64_t cache_size; > > API returns an int so a -ve return value would overflow and pass the > assert below. Will fix > > > + > > + cache_size = odp_sys_cache_line_size(); > > + CU_ASSERT(0 < cache_size) > > +} > > + > > +static void test_odp_sys_cpu_model_str(void) > > +{ > > + char model[128]; > > + > > + strcpy(model, odp_sys_cpu_model_str()); > > This may overrun as you've no idea how long the string is, and is it OK > for the function to return NULL or an empty string? > > The API does not specify, but linux-generic allocates 128 chars and assigns 0 to the last byte. If we are ok with it I will test for that specification and create a patch to update the docs. > > + CU_ASSERT(strlen(model) > 0) > > + CU_ASSERT(strlen(model) < 127) > > +} > > + > > +static void test_odp_sys_page_size(void) > > +{ > > + uint64_t page; > > + > > + page = odp_sys_page_size(); > > + CU_ASSERT(0 < page) > > +} > > + > > +static void test_odp_sys_huge_page_size(void) > > +{ > > + uint64_t page; > > + > > + page = odp_sys_huge_page_size(); > > + CU_ASSERT(0 < page) > > Difficult to know what should be tested here as the API isn't well > defined, perhaps 0 would mean there are no hugepages which I think > should be valid. > I can update the docs with the clarification > > I suppose you could also check that it's either 0 or >= odp_sys_page_size() > Do we need to do that, don't we trust the implementation on the actual value as long as it is a legal number? i.e. Are we saying that on all systems you must have larger or equal hugepages ? makes sense but should we enforce it ? > > (although surely the page size APIs should be made internal.) > It is in the delta doc as a suggestion for completion by 1.0 > > > +} > > + > > +static void test_odp_sys_cpu_hz(void) > > +{ > > + uint64_t hz; > > + > > + hz = odp_sys_cpu_hz(); > > + CU_ASSERT(0 < hz) > > +} > > + > > +CU_TestInfo test_odp_system[] = { > > + {"odp_sys_core_count", test_odp_sys_core_count}, > > + {"odp_sys_cache_line_size", test_odp_sys_cache_line_size}, > > + {"odp_sys_cpu_model_str", test_odp_sys_cpu_model_str}, > > + {"odp_sys_page_size", test_odp_sys_page_size}, > > + {"odp_sys_huge_page_size", test_odp_sys_huge_page_size}, > > + {"odp_sys_cpu_hz", test_odp_sys_cpu_hz}, > > + CU_TEST_INFO_NULL, > > +}; > > + > > +CU_SuiteInfo odp_testsuites[] = { > > + {"System Info", NULL, NULL, NULL, NULL, > > + test_odp_system}, > > + CU_SUITE_INFO_NULL, > > +}; > > -- > > 2.1.0 > > > > -- > Stuart. > > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
