From 21b37d4ee6d9ee029b881e831d2f5e4916957545 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Mon, 28 Sep 2015 10:08:37 -0700 Subject: [PATCH] nvme: make the unit test assert actually assert The current implementation of nvme_assert in the unit test nvme_impl.h just prints the message and continues. We should not be triggering assert conditions, even in the unit test code, so make nvme_assert actually call assert(). This lets us catch mistakes in the unit tests more easily. Also fix the two unit tests that currently trigger an assert: - The I/O splitting test in nvme_ns_cmd_ut was passing an invalid combination of NULL payload with non-zero lba_count. - The ctrlr_cmd test was passing an invalid number of entries to nvme_ctrlr_cmd_get_error_page(). This case should probably not be an assert but rather an error code. However, the function does not return a status code currently, so it is not trivial to make that change. For now, just drop the asserting test case and the code added to the test to work around it. While we're here, fix the macros in the unit test nvme_impl.h so they are usable in single-line conditionals without braces - that is the whole point of the do { ... } while (0) pattern, so there should be no trailing semicolon. Change-Id: Iad503c5c5d19a426d48c80d9a7d6da12ff2c982a Signed-off-by: Daniel Verkamp --- .../nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c | 7 ------- test/lib/nvme/unit/nvme_impl.h | 12 ++++++++---- test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c | 3 ++- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c b/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c index cd7ae46e7..77b9aecaa 100644 --- a/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c +++ b/test/lib/nvme/unit/nvme_ctrlr_cmd_c/nvme_ctrlr_cmd_ut.c @@ -86,9 +86,6 @@ void verify_error_log_page(struct nvme_request *req) CU_ASSERT(req->cmd.opc == NVME_OPC_GET_LOG_PAGE); CU_ASSERT(req->cmd.nsid == NVME_GLOBAL_NAMESPACE_TAG); - if (error_num_entries > CTRLR_CDATA_ELPE + 1) - error_num_entries = CTRLR_CDATA_ELPE + 1; - temp_cdw10 = (((sizeof(struct nvme_error_information_entry) * error_num_entries) / sizeof( uint32_t) - 1) << 16) | NVME_LOG_ERROR; CU_ASSERT(req->cmd.cdw10 == temp_cdw10); @@ -197,10 +194,6 @@ test_error_get_log_page() /* valid page */ error_num_entries = 1; nvme_ctrlr_cmd_get_error_page(&ctrlr, &payload, error_num_entries, NULL, NULL); - - /* out of range page */ - error_num_entries = 50; - nvme_ctrlr_cmd_get_error_page(&ctrlr, &payload, error_num_entries, NULL, NULL); } void diff --git a/test/lib/nvme/unit/nvme_impl.h b/test/lib/nvme/unit/nvme_impl.h index 11366371b..ca6083fb8 100644 --- a/test/lib/nvme/unit/nvme_impl.h +++ b/test/lib/nvme/unit/nvme_impl.h @@ -34,6 +34,7 @@ #ifndef __NVME_IMPL_H__ #define __NVME_IMPL_H__ +#include #include #include #include @@ -56,17 +57,20 @@ extern char outbuf[OUTBUF_SIZE]; #define nvme_assert(check, str) \ do \ { \ - if (!(check)) \ - printf str; \ + if (!(check)) { \ + printf str; \ + assert(check); \ + } \ } \ - while (0); + while (0) + uint64_t nvme_vtophys(void *buf); #define nvme_alloc_request(bufp) \ do \ { \ *bufp = malloc(sizeof(struct nvme_request)); \ } \ - while (0); + while (0) #define nvme_free_request(buf) free(buf) #define nvme_pcicfg_read32(handle, var, offset) do { *(var) = 0xFFFFFFFFu; } while (0) diff --git a/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c b/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c index 7ede51335..1245ee357 100644 --- a/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c +++ b/test/lib/nvme/unit/nvme_ns_cmd_c/nvme_ns_cmd_ut.c @@ -113,7 +113,7 @@ split_test(void) int rc; prepare_for_test(&ns, &ctrlr, 512, 128 * 1024, 0); - payload = 0x0; + payload = malloc(512); lba = 0; lba_count = 1; @@ -126,6 +126,7 @@ split_test(void) } CU_ASSERT(g_request->num_children == 0); + free(payload); nvme_free_request(g_request); }