[PATCH] cpukit/include/dev/can: Disabled debug prints in CAN Framework

[Gedare Bloom]

Hi Prashanth,

On Mon, Nov 28, 2022 at 8:46 PM Prashanth S <fishesprashanth at gmail.com> wrote:
>
> ---
>  cpukit/include/dev/can/can.h     |  2 +-
>  testsuites/libtests/can01/init.c | 14 ++++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/cpukit/include/dev/can/can.h b/cpukit/include/dev/can/can.h
> index 9e55395039..27afd00088 100644
> --- a/cpukit/include/dev/can/can.h
> +++ b/cpukit/include/dev/can/can.h
> @@ -53,7 +53,7 @@
>        printf(str, ##__VA_ARGS__);                                             \
>      } while (false);
>
> -#define CAN_DEBUG(str, ...) DEBUG(str, ##__VA_ARGS__)
> +#define CAN_DEBUG(str, ...) //DEBUG(str, ##__VA_ARGS__)

This is not the right way to disable this, for several reasons.

I should have caught this DEBUG printing stuff earlier. My guess is
that you should just remove it all from the production code, or it
should be controlled at a higher level with -DDEBUG kind of flag.
However, I agree that the current implementation is overly verbose in
debug mode. I'd just strip it all out.

>  #define CAN_DEBUG_BUF(str, ...) CAN_DEBUG(str, ##__VA_ARGS__)
>  #define CAN_DEBUG_ISR(str, ...) CAN_DEBUG(str, ##__VA_ARGS__)
>  #define CAN_DEBUG_LOCK(str, ...) CAN_DEBUG(str, ##__VA_ARGS__)
> diff --git a/testsuites/libtests/can01/init.c b/testsuites/libtests/can01/init.c
> index 0675fe606f..c19d45602e 100644
> --- a/testsuites/libtests/can01/init.c
> +++ b/testsuites/libtests/can01/init.c
> @@ -97,8 +97,10 @@ static void test_task(rtems_task_argument data)
>
>    rtems_test_assert(fd >= 0);
>
> +  printf("test_task %u\n", task_num);
> +
>    for (int i = 0; i < NUM_TEST_MSGS; i++) {
> -    printf("test_task %u\n", task_num);
> +    CAN_DEBUG("test_task %u\n", task_num);
>
>      msg.id = task_num;
>      //FIXME: Implement Test cases for other flags also.

There shouldn't be // comments anywhere.

> @@ -111,18 +113,18 @@ static void test_task(rtems_task_argument data)
>
>      msg_size = ((char *)&msg.data[msg.len] - (char *)&msg);
>
> -    printf("calling write task = %u\n", task_num);
> +    CAN_DEBUG("calling write task = %u\n", task_num);

The general philosophy in our test suite is to only print something if
there's a failure. So just remove these debug statements that are
leftover from your development approach.

>
>      count = write(fd, &msg, sizeof(msg));
>      rtems_test_assert(count == msg_size);
> -    printf("task = %u write count = %u\n", task_num, count);
> +    CAN_DEBUG("task = %u write count = %u\n", task_num, count);
>
> -    printf("calling read task = %u\n", task_num);
> +    CAN_DEBUG("calling read task = %u\n", task_num);
>      count = read(fd, &msg, sizeof(msg));
>      rtems_test_assert(count > 0);
Since you send the messages, you know their sizes. You should be able
to assert the exact amount received?

> -    printf("task = %u read count = %u\n", task_num, count);
> +    CAN_DEBUG("task = %u read count = %u\n", task_num, count);
>
> -    printf("received message\n");
> +    CAN_DEBUG("received message\n");
>      can_print_msg(&msg);
>
>      sleep(1);
> --
> 2.25.1
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel