Re: [lng-odp] [PATCH 3/5] validation: cosmetic changes in odp_timer.c

2015-07-08 Thread Stuart Haslam 
On Fri, Jul 03, 2015 at 05:45:38PM +0200, Christophe Milard wrote:
 As preparation before moving the file (next patch) where the whole
 file will be checked again by check-patch called via check-odp.
 Some warnings remains about missing blank lines, after variable
 declarations, but it would not add to readibility when the
 variables are declared in the function's body.
 Also a few line over 80 char warning remains, but these lines includes
 strings which should not be split.
 
 Signed-off-by: Christophe Milard christophe.mil...@linaro.org
 ---
  test/validation/odp_timer.c | 37 -
  1 file changed, 20 insertions(+), 17 deletions(-)
 
 diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c
 index 3ab395d..9efb360 100644
 --- a/test/validation/odp_timer.c
 +++ b/test/validation/odp_timer.c
 @@ -225,25 +225,26 @@ static void handle_tmo(odp_event_t ev, bool stale, 
 uint64_t prev_tick)
  
   if (tim == ODP_TIMER_INVALID)
   CU_FAIL(odp_timeout_timer() invalid timer);
 - if (ttp == NULL)
 + if (!ttp)
   CU_FAIL(odp_timeout_user_ptr() null user ptr);
  
 - if (ttp != NULL  ttp-ev2 != ev)
 + if (ttp  ttp-ev2 != ev)
   CU_FAIL(odp_timeout_user_ptr() wrong user ptr);
 - if (ttp != NULL  ttp-tim != tim)
 + if (ttp  ttp-tim != tim)
   CU_FAIL(odp_timeout_timer() wrong timer);
   if (stale) {
   if (odp_timeout_fresh(tmo))
   CU_FAIL(Wrong status (fresh) for stale timeout);
   /* Stale timeout = local timer must have invalid tick */
 - if (ttp != NULL  ttp-tick != TICK_INVALID)
 + if (ttp  ttp-tick != TICK_INVALID)
   CU_FAIL(Stale timeout for active timer);
   } else {
   if (!odp_timeout_fresh(tmo))
   CU_FAIL(Wrong status (stale) for fresh timeout);
   /* Fresh timeout = local timer must have matching tick */
 - if (ttp != NULL  ttp-tick != tick) {
 - LOG_DBG(Wrong tick: expected %PRIu64 actual 
 %PRIu64\n,
 + if (ttp  ttp-tick != tick) {
 + LOG_DBG(Wrong tick: expected % PRIu64
 +  actual % PRIu64 \n,

I think this would be better off left unwrapped.

   ttp-tick, tick);
   CU_FAIL(odp_timeout_tick() wrong tick);
   }
 @@ -251,14 +252,15 @@ static void handle_tmo(odp_event_t ev, bool stale, 
 uint64_t prev_tick)
   if (tick  odp_timer_current_tick(tp))
   CU_FAIL(Timeout delivered early);
   if (tick  prev_tick) {
 - LOG_DBG(Too late tick: %PRIu64 prev_tick 
 %PRIu64\n,
 + LOG_DBG(Too late tick: % PRIu64
 +  prev_tick % PRIu64\n,

And here.

I thought checkpatch had been modified to ignore long lines for debug
macros, but perhaps this isn't working as it does WARN on another
LOG_DBG further down..

   tick, prev_tick);
   /* We don't report late timeouts using CU_FAIL */
   odp_atomic_inc_u32(ndelivtoolate);
   }
   }
  
 - if (ttp != NULL) {
 + if (ttp) {
   /* Internal error */
   CU_ASSERT_FATAL(ttp-ev == ODP_EVENT_INVALID);
   ttp-ev = ev;
 @@ -281,7 +283,7 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
   CU_FAIL_FATAL(Queue create failed);
  
   struct test_timer *tt = malloc(sizeof(struct test_timer) * NTIMERS);
 - if (tt == NULL)
 + if (!tt)
   CU_FAIL_FATAL(malloc failed);
  
   /* Prepare all timers */
 @@ -322,6 +324,7 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
   uint32_t ntoolate = 0;
   uint32_t ms;
   uint64_t prev_tick = odp_timer_current_tick(tp);
 +
   for (ms = 0; ms  7 * RANGE_MS / 10; ms++) {
   odp_event_t ev;
   while ((ev = odp_queue_deq(queue)) != ODP_EVENT_INVALID) {
 @@ -390,13 +393,13 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
   CU_FAIL(odp_timer_free);
   }
  
 - LOG_DBG(Thread %u: %PRIu32 timers set\n, thr, nset);
 - LOG_DBG(Thread %u: %PRIu32 timers reset\n, thr, nreset);
 - LOG_DBG(Thread %u: %PRIu32 timers cancelled\n, thr, ncancel);
 - LOG_DBG(Thread %u: %PRIu32 timers reset/cancelled too late\n,
 + LOG_DBG(Thread %u: % PRIu32  timers set\n, thr, nset);
 + LOG_DBG(Thread %u: % PRIu32  timers reset\n, thr, nreset);
 + LOG_DBG(Thread %u: % PRIu32  timers cancelled\n, thr, ncancel);
 + LOG_DBG(Thread %u: % PRIu32  timers reset/cancelled too late\n,
   thr, ntoolate);
 - LOG_DBG(Thread %u: %PRIu32 timeouts received\n, thr, nrcv);
 - LOG_DBG(Thread %u: %PRIu32 stale timeout(s) after 
 odp_timer_free()\n,
 + LOG_DBG(Thread %u: % PRIu32

Re: [lng-odp] [PATCH 3/5] validation: cosmetic changes in odp_timer.c

2015-07-08 Thread Christophe Milard 
It did warn me. This is the reason why I broke some of these lines when it
looked reasonable. What do you wish me to do?
Christophe.

On 8 July 2015 at 14:31, Stuart Haslam stuart.has...@linaro.org wrote:

 On Fri, Jul 03, 2015 at 05:45:38PM +0200, Christophe Milard wrote:
  As preparation before moving the file (next patch) where the whole
  file will be checked again by check-patch called via check-odp.
  Some warnings remains about missing blank lines, after variable
  declarations, but it would not add to readibility when the
  variables are declared in the function's body.
  Also a few line over 80 char warning remains, but these lines includes
  strings which should not be split.
 
  Signed-off-by: Christophe Milard christophe.mil...@linaro.org
  ---
   test/validation/odp_timer.c | 37 -
   1 file changed, 20 insertions(+), 17 deletions(-)
 
  diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c
  index 3ab395d..9efb360 100644
  --- a/test/validation/odp_timer.c
  +++ b/test/validation/odp_timer.c
  @@ -225,25 +225,26 @@ static void handle_tmo(odp_event_t ev, bool stale,
 uint64_t prev_tick)
 
if (tim == ODP_TIMER_INVALID)
CU_FAIL(odp_timeout_timer() invalid timer);
  - if (ttp == NULL)
  + if (!ttp)
CU_FAIL(odp_timeout_user_ptr() null user ptr);
 
  - if (ttp != NULL  ttp-ev2 != ev)
  + if (ttp  ttp-ev2 != ev)
CU_FAIL(odp_timeout_user_ptr() wrong user ptr);
  - if (ttp != NULL  ttp-tim != tim)
  + if (ttp  ttp-tim != tim)
CU_FAIL(odp_timeout_timer() wrong timer);
if (stale) {
if (odp_timeout_fresh(tmo))
CU_FAIL(Wrong status (fresh) for stale timeout);
/* Stale timeout = local timer must have invalid tick */
  - if (ttp != NULL  ttp-tick != TICK_INVALID)
  + if (ttp  ttp-tick != TICK_INVALID)
CU_FAIL(Stale timeout for active timer);
} else {
if (!odp_timeout_fresh(tmo))
CU_FAIL(Wrong status (stale) for fresh timeout);
/* Fresh timeout = local timer must have matching tick */
  - if (ttp != NULL  ttp-tick != tick) {
  - LOG_DBG(Wrong tick: expected %PRIu64 actual
 %PRIu64\n,
  + if (ttp  ttp-tick != tick) {
  + LOG_DBG(Wrong tick: expected % PRIu64
  +  actual % PRIu64 \n,

 I think this would be better off left unwrapped.

ttp-tick, tick);
CU_FAIL(odp_timeout_tick() wrong tick);
}
  @@ -251,14 +252,15 @@ static void handle_tmo(odp_event_t ev, bool stale,
 uint64_t prev_tick)
if (tick  odp_timer_current_tick(tp))
CU_FAIL(Timeout delivered early);
if (tick  prev_tick) {
  - LOG_DBG(Too late tick: %PRIu64 prev_tick
 %PRIu64\n,
  + LOG_DBG(Too late tick: % PRIu64
  +  prev_tick % PRIu64\n,

 And here.

 I thought checkpatch had been modified to ignore long lines for debug
 macros, but perhaps this isn't working as it does WARN on another
 LOG_DBG further down..

tick, prev_tick);
/* We don't report late timeouts using CU_FAIL */
odp_atomic_inc_u32(ndelivtoolate);
}
}
 
  - if (ttp != NULL) {
  + if (ttp) {
/* Internal error */
CU_ASSERT_FATAL(ttp-ev == ODP_EVENT_INVALID);
ttp-ev = ev;
  @@ -281,7 +283,7 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
CU_FAIL_FATAL(Queue create failed);
 
struct test_timer *tt = malloc(sizeof(struct test_timer) *
 NTIMERS);
  - if (tt == NULL)
  + if (!tt)
CU_FAIL_FATAL(malloc failed);
 
/* Prepare all timers */
  @@ -322,6 +324,7 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
uint32_t ntoolate = 0;
uint32_t ms;
uint64_t prev_tick = odp_timer_current_tick(tp);
  +
for (ms = 0; ms  7 * RANGE_MS / 10; ms++) {
odp_event_t ev;
while ((ev = odp_queue_deq(queue)) != ODP_EVENT_INVALID) {
  @@ -390,13 +393,13 @@ static void *worker_entrypoint(void *arg
 TEST_UNUSED)
CU_FAIL(odp_timer_free);
}
 
  - LOG_DBG(Thread %u: %PRIu32 timers set\n, thr, nset);
  - LOG_DBG(Thread %u: %PRIu32 timers reset\n, thr, nreset);
  - LOG_DBG(Thread %u: %PRIu32 timers cancelled\n, thr, ncancel);
  - LOG_DBG(Thread %u: %PRIu32 timers reset/cancelled too late\n,
  + LOG_DBG(Thread %u: % PRIu32  timers set\n, thr, nset);
  + LOG_DBG(Thread %u: % PRIu32  timers reset\n, thr, nreset);
  + LOG_DBG(Thread %u: % PRIu32  timers cancelled\n, thr,

Re: [lng-odp] [PATCH 3/5] validation: cosmetic changes in odp_timer.c

2015-07-08 Thread Stuart Haslam 
On Wed, Jul 08, 2015 at 02:37:01PM +0200, Christophe Milard wrote:
 It did warn me. This is the reason why I broke some of these lines when it
 looked reasonable. What do you wish me to do?

Not wrap the line in the two places I highlighted.

As a secondary thing we should also look into why checkpatch is warning
on these as it shouldn't be AFAICT (I did have a quick look but the
regex there made my eyes bleed).

--
Stuart.
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp