I'm all for keeping things simple and just using printf() or other standard C library functions in validation tests. The purpose of the macros in ODP code was to allow for transparent redirection of log messages to an application-supplied logger, and that's not something that we need for the validation tests.
The same could be said for the similar macros used in the examples, but I see no urgency to make changes there. On Mon, Feb 6, 2017 at 11:12 AM, Mike Holmes <mike.hol...@linaro.org> wrote: > Call for objections to removing the macro ? > > Sounds reasonable to me, and I dont think we need to block this going > in, cleaning up is a seperate issue so I will make a bug for it > > On 6 February 2017 at 10:11, Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolai...@nokia-bell-labs.com> wrote: >> >> >>> -----Original Message----- >>> From: Maxim Uvarov [mailto:maxim.uva...@linaro.org] >>> Sent: Monday, February 06, 2017 5:05 PM >>> To: Mike Holmes <mike.hol...@linaro.org> >>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell- >>> labs.com>; lng-odp <lng-odp@lists.linaro.org> >>> Subject: Re: [lng-odp] [PATCH 05/10] validation: packet: print reason for >>> suite init failure >>> >>> On 02/06/17 18:01, Mike Holmes wrote: >>> > On 6 February 2017 at 09:55, Maxim Uvarov <maxim.uva...@linaro.org> >>> wrote: >>> >> On 02/06/17 17:49, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> >>> >>> >>> >>> >>>> -----Original Message----- >>> >>>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of >>> Mike >>> >>>> Holmes >>> >>>> Sent: Monday, February 06, 2017 4:41 PM >>> >>>> To: Maxim Uvarov <maxim.uva...@linaro.org> >>> >>>> Cc: lng-odp <lng-odp@lists.linaro.org> >>> >>>> Subject: Re: [lng-odp] [PATCH 05/10] validation: packet: print reason >>> for >>> >>>> suite init failure >>> >>>> >>> >>>> On 6 February 2017 at 09:34, Maxim Uvarov <maxim.uva...@linaro.org> >>> wrote: >>> >>>>> On 02/06/17 15:37, Petri Savolainen wrote: >>> >>>>>> Knowing the reason for suite init function failure helps in >>> >>>>>> debugging. >>> >>>>>> >>> >>>>>> Signed-off-by: Petri Savolainen <petri.savolai...@linaro.org> >>> >>>>>> --- >>> >>>>>> test/common_plat/validation/api/packet/packet.c | 23 >>> >>>> ++++++++++++++++++----- >>> >>>>>> 1 file changed, 18 insertions(+), 5 deletions(-) >>> >>>>>> >>> >>>>>> diff --git a/test/common_plat/validation/api/packet/packet.c >>> >>>> b/test/common_plat/validation/api/packet/packet.c >>> >>>>>> index fa5206f..e3d28f6 100644 >>> >>>>>> --- a/test/common_plat/validation/api/packet/packet.c >>> >>>>>> +++ b/test/common_plat/validation/api/packet/packet.c >>> >>>>>> @@ -110,8 +110,10 @@ int packet_suite_init(void) >>> >>>>>> uint8_t data = 0; >>> >>>>>> uint32_t i; >>> >>>>>> >>> >>>>>> - if (odp_pool_capability(&capa) < 0) >>> >>>>>> + if (odp_pool_capability(&capa) < 0) { >>> >>>>>> + printf("pool_capability failed\n"); >>> >>>> >>> >>>> We have defined LOG_DBG in test_debug.h, shoudl we be using that ? >>> >>>> >>> >>> >>> >>> All other xxx_suite_init() just use printf() or don't print at all. >>> This is just applying the current practice. >>> >>> >>> >>> -Petri >>> >>> >>> >>> >>> >> >>> >> LOG_ for implementation only, not for tests. >>> > >>> > That is not true currently, but happy if we delete the current cases >>> > or document why we pick either method. >>> > >>> > common_plat/validation/api/system/system.c: >>> > LOG_DBG("\nBAD VERSION=%s\n", version_string); >>> > common_plat/validation/api/timer/timer.c: LOG_DBG("Timer handle: >>> > %" PRIu64 "\n", odp_timer_to_u64(tim)); >>> > common_plat/validation/api/timer/timer.c: LOG_DBG("Timeout >>> > handle: %" PRIu64 "\n", odp_timeout_to_u64(tmo)); >>> > common_plat/validation/api/timer/timer.c: >>> > LOG_DBG("Wrong tick: expected %" PRIu64 >>> > common_plat/validation/api/timer/timer.c: >>> > LOG_DBG("Too late tick: %" PRIu64 >>> > common_plat/validation/api/timer/timer.c: >>> > LOG_DBG("Failed to allocate timeout (%" PRIu32 "/%d)\n", >>> > common_plat/validation/api/timer/timer.c: >>> > LOG_DBG("Failed to allocate timer >>> > .... >>> > >>> > >>> > I think we should be consistent, looks like there are two standards, >>> > some with printf >>> >>> >>> unbelievable LOG_ are defined in test/test_debug.h >>> In that case we should use them in tests. >>> >>> Maxim. >> >> Found over 200 hits of printf() in current validation test .c files. May be >> someone can take an action to clean out all of those. This patch just >> follows the current practice of xxx_suite_init() functions. >> >> -Petri >> > > > > -- > Mike Holmes > Program Manager - Linaro Networking Group > Linaro.org │ Open source software for ARM SoCs > "Work should be fun and collaborative, the rest follows"
