test: forbid use of CU_ASSERT_FATAL

Static analyzers don't see CU_ASSERT_FATAL as truly fatal,
and will complain that code later in the function may try
to dereference a NULL pointer.  So we added SPDK_CU_ASSERT_FATAL
which should be used instead.

This still trips people up sometimes though - the static analyzer
complains and then the developer will add other checks that
pointers are not NULL.

So instead, forbid use of CU_ASSERT_FATAL through check_format.sh
and explain why.  While here, fix up all of the existing CU_ASSERT_FATAL
usages to either CU_ASSERT or SPDK_CU_ASSERT_FATAL.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I7974c8c85ddb89ed1b7d882db3a2eb0882ea0217

Reviewed-on: https://review.gerrithub.io/418111
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Seth Howell <seth.howell5141@gmail.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
This commit is contained in:
Jim Harris 2018-07-05 05:52:19 -07:00
parent 20836de289
commit 56d35c5d13
6 changed files with 21 additions and 9 deletions

View File

@ -87,6 +87,18 @@ else
fi
rm -f badfunc.log
echo -n "Checking for use of forbidden CUnit macros..."
git grep --line-number -w 'CU_ASSERT_FATAL' -- 'test/*' ':!test/spdk_cunit.h' > badcunit.log || true
if [ -s badcunit.log ]; then
echo " Forbidden CU_ASSERT_FATAL usage detected - use SPDK_CU_ASSERT_FATAL instead"
cat badcunit.log
rc=1
else
echo " OK"
fi
rm -f badcunit.log
echo -n "Checking blank lines at end of file..."
if ! git grep -I -l -e . -z | \

View File

@ -197,7 +197,7 @@ maxburstlength_test(void)
req->final_bit = 1;
rc = spdk_iscsi_execute(&conn, req_pdu);
CU_ASSERT_FATAL(rc == 0);
CU_ASSERT(rc == 0);
response_pdu = TAILQ_FIRST(&g_write_pdu_list);
SPDK_CU_ASSERT_FATAL(response_pdu != NULL);

View File

@ -34,7 +34,7 @@
#include "spdk/stdinc.h"
#include "spdk/event.h"
#include "CUnit/Basic.h"
#include "spdk_cunit.h"
#include "../common.c"
#include "iscsi/portal_grp.c"
@ -174,7 +174,7 @@ parse_portal_ipv4_normal_case(void)
int rc;
cpumask_val = spdk_cpuset_alloc();
CU_ASSERT_FATAL(cpumask_val != NULL);
SPDK_CU_ASSERT_FATAL(cpumask_val != NULL);
spdk_cpuset_set_cpu(cpumask_val, 0, true);
@ -201,7 +201,7 @@ parse_portal_ipv6_normal_case(void)
int rc;
cpumask_val = spdk_cpuset_alloc();
CU_ASSERT_FATAL(cpumask_val != NULL);
SPDK_CU_ASSERT_FATAL(cpumask_val != NULL);
spdk_cpuset_set_cpu(cpumask_val, 0, true);

View File

@ -62,7 +62,7 @@ write_cb(void *cb_ctx, const void *data, size_t size)
memset(g_buf, 0, sizeof(g_buf)); \
g_write_pos = g_buf; \
w = spdk_json_write_begin(write_cb, NULL, 0); \
CU_ASSERT_FATAL(w != NULL)
SPDK_CU_ASSERT_FATAL(w != NULL)
#define END(json) \
CU_ASSERT(spdk_json_write_end(w) == 0); \

View File

@ -447,7 +447,7 @@ test_spdk_nvme_detach(void)
g_spdk_nvme_driver = &test_driver;
TAILQ_INIT(&test_driver.shared_attached_ctrlrs);
TAILQ_INSERT_TAIL(&test_driver.shared_attached_ctrlrs, &ctrlr, tailq);
CU_ASSERT_FATAL(pthread_mutex_init(&test_driver.lock, NULL) == 0);
CU_ASSERT(pthread_mutex_init(&test_driver.lock, NULL) == 0);
/*
* Controllers are ref counted so mock the function that returns

View File

@ -211,7 +211,7 @@ spdk_nvme_ctrlr_cmd_set_feature(struct spdk_nvme_ctrlr *ctrlr, uint8_t feature,
uint32_t cdw11, uint32_t cdw12, void *payload, uint32_t payload_size,
spdk_nvme_cmd_cb cb_fn, void *cb_arg)
{
CU_ASSERT_FATAL(0);
CU_ASSERT(0);
return -1;
}
@ -220,7 +220,7 @@ spdk_nvme_ctrlr_cmd_get_feature(struct spdk_nvme_ctrlr *ctrlr, uint8_t feature,
uint32_t cdw11, void *payload, uint32_t payload_size,
spdk_nvme_cmd_cb cb_fn, void *cb_arg)
{
CU_ASSERT_FATAL(0);
CU_ASSERT(0);
return -1;
}
@ -1124,7 +1124,7 @@ setup_qpairs(struct spdk_nvme_ctrlr *ctrlr, uint32_t num_io_queues)
{
uint32_t i;
CU_ASSERT_FATAL(pthread_mutex_init(&ctrlr->ctrlr_lock, NULL) == 0);
CU_ASSERT(pthread_mutex_init(&ctrlr->ctrlr_lock, NULL) == 0);
SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(ctrlr) == 0);