[bug report] kunit: test: add support for test abort

[bug report] kunit: test: add support for test abort

[Dan Carpenter]

Hello Brendan Higgins,

The patch 5f3e06208920: "kunit: test: add support for test abort"
from Sep 23, 2019, leads to the following static checker warning:

	lib/kunit/try-catch.c:93 kunit_try_catch_run()
	misplaced newline? '    # %s: Unknown error: %d

lib/kunit/try-catch.c
    58  void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
    59  {
    60          DECLARE_COMPLETION_ONSTACK(try_completion);
    61          struct kunit *test = try_catch->test;
    62          struct task_struct *task_struct;
    63          int exit_code, time_remaining;
    64  
    65          try_catch->context = context;
    66          try_catch->try_completion = &try_completion;
    67          try_catch->try_result = 0;
    68          task_struct = kthread_run(kunit_generic_run_threadfn_adapter,
    69                                    try_catch,
    70                                    "kunit_try_catch_thread");
    71          if (IS_ERR(task_struct)) {
    72                  try_catch->catch(try_catch->context);
    73                  return;
    74          }
    75  
    76          time_remaining = wait_for_completion_timeout(&try_completion,
    77                                                       kunit_test_timeout());
    78          if (time_remaining == 0) {
    79                  kunit_err(test, "try timed out\n");
                                                      ^^
The kunit_log() macro adds its own newline.  Most of the callers add
a newline.  It should be the callers add a newline because that's how
everything else works in the kernel.

The dev_printk() stuff will sometimes add a newline, but never a
duplicate newline.  In other words, it's slightly complicated.  But
basically the caller should add a newline.

    80                  try_catch->try_result = -ETIMEDOUT;
    81          }
    82  
    83          exit_code = try_catch->try_result;
    84  
    85          if (!exit_code)
    86                  return;
    87  
    88          if (exit_code == -EFAULT)
    89                  try_catch->try_result = 0;
    90          else if (exit_code == -EINTR)
    91                  kunit_err(test, "wake_up_process() was never called\n");
                                                                           ^^

    92          else if (exit_code)
    93                  kunit_err(test, "Unknown error: %d\n", exit_code);
                                                          ^^

    94  
    95          try_catch->catch(try_catch->context);
    96  }

regards,
dan carpenter

Re: [bug report] kunit: test: add support for test abort

[Brendan Higgins]

On Sat, Apr 18, 2020 at 10:26 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Brendan Higgins,
>
> The patch 5f3e06208920: "kunit: test: add support for test abort"
> from Sep 23, 2019, leads to the following static checker warning:
>
>         lib/kunit/try-catch.c:93 kunit_try_catch_run()
>         misplaced newline? '    # %s: Unknown error: %d
>
> lib/kunit/try-catch.c
>     58  void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
>     59  {
>     60          DECLARE_COMPLETION_ONSTACK(try_completion);
>     61          struct kunit *test = try_catch->test;
>     62          struct task_struct *task_struct;
>     63          int exit_code, time_remaining;
>     64
>     65          try_catch->context = context;
>     66          try_catch->try_completion = &try_completion;
>     67          try_catch->try_result = 0;
>     68          task_struct = kthread_run(kunit_generic_run_threadfn_adapter,
>     69                                    try_catch,
>     70                                    "kunit_try_catch_thread");
>     71          if (IS_ERR(task_struct)) {
>     72                  try_catch->catch(try_catch->context);
>     73                  return;
>     74          }
>     75
>     76          time_remaining = wait_for_completion_timeout(&try_completion,
>     77                                                       kunit_test_timeout());
>     78          if (time_remaining == 0) {
>     79                  kunit_err(test, "try timed out\n");
>                                                       ^^
> The kunit_log() macro adds its own newline.  Most of the callers add
> a newline.  It should be the callers add a newline because that's how
> everything else works in the kernel.

Whoops, I thought I removed that extra newline.

Thanks! I will look into this.

> The dev_printk() stuff will sometimes add a newline, but never a
> duplicate newline.  In other words, it's slightly complicated.  But
> basically the caller should add a newline.
>
>     80                  try_catch->try_result = -ETIMEDOUT;
>     81          }
>     82
>     83          exit_code = try_catch->try_result;
>     84
>     85          if (!exit_code)
>     86                  return;
>     87
>     88          if (exit_code == -EFAULT)
>     89                  try_catch->try_result = 0;
>     90          else if (exit_code == -EINTR)
>     91                  kunit_err(test, "wake_up_process() was never called\n");
>                                                                            ^^
>
>     92          else if (exit_code)
>     93                  kunit_err(test, "Unknown error: %d\n", exit_code);
>                                                           ^^
>
>     94
>     95          try_catch->catch(try_catch->context);
>     96  }
>
> regards,
> dan carpenter