From 3fa7c33ac12075db11763e31564d2479ec38f4e5 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 19 Apr 2018 13:22:55 -0700 Subject: [PATCH] nvme: require trid to be valid in nvme_ctrlr_probe This is an internal NVMe driver function, so we don't need to allow for the case where trid is NULL. All callers already passed an address of a local variable except the unit tests, which can be trivially fixed. Fixes a static analyzer warning about trid being dereferenced in nvme_transport_ctrlr_construct() before being checked for NULL in the caller. Change-Id: I2bfeb5c92a302093b7c7f2949adcd18baa11855a Signed-off-by: Daniel Verkamp Reviewed-on: https://review.gerrithub.io/408395 Reviewed-by: Ben Walker Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris --- lib/nvme/nvme.c | 8 +++----- test/unit/lib/nvme/nvme.c/nvme_ut.c | 8 ++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index a141d287b7..123dc8a4ca 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -342,16 +342,14 @@ nvme_ctrlr_probe(const struct spdk_nvme_transport_id *trid, void *devhandle, struct spdk_nvme_ctrlr *ctrlr; struct spdk_nvme_ctrlr_opts opts; + assert(trid != NULL); + spdk_nvme_ctrlr_get_default_ctrlr_opts(&opts, sizeof(opts)); if (!probe_cb || probe_cb(cb_ctx, trid, &opts)) { ctrlr = nvme_transport_ctrlr_construct(trid, &opts, devhandle); if (ctrlr == NULL) { - if (trid != NULL) { - SPDK_ERRLOG("Failed to construct NVMe controller for SSD: %s\n", trid->traddr); - } else { - SPDK_ERRLOG("Failed to construct NVMe controller\n"); - } + SPDK_ERRLOG("Failed to construct NVMe controller for SSD: %s\n", trid->traddr); return -1; } diff --git a/test/unit/lib/nvme/nvme.c/nvme_ut.c b/test/unit/lib/nvme/nvme.c/nvme_ut.c index 27b683b67b..bfc7b50572 100644 --- a/test/unit/lib/nvme/nvme.c/nvme_ut.c +++ b/test/unit/lib/nvme/nvme.c/nvme_ut.c @@ -730,21 +730,21 @@ static void test_nvme_ctrlr_probe(void) { int rc = 0; - const struct spdk_nvme_transport_id *trid = NULL; + const struct spdk_nvme_transport_id trid = {}; void *devhandle = NULL; void *cb_ctx = NULL; struct spdk_nvme_ctrlr *dummy = NULL; /* test when probe_cb returns false */ MOCK_SET(dummy_probe_cb, bool, false); - rc = nvme_ctrlr_probe(trid, devhandle, dummy_probe_cb, cb_ctx); + rc = nvme_ctrlr_probe(&trid, devhandle, dummy_probe_cb, cb_ctx); CU_ASSERT(rc == 1); /* probe_cb returns true but we can't construct a ctrl */ MOCK_SET(dummy_probe_cb, bool, true); MOCK_SET_P(nvme_transport_ctrlr_construct, struct spdk_nvme_ctrlr *, NULL); - rc = nvme_ctrlr_probe(trid, devhandle, dummy_probe_cb, cb_ctx); + rc = nvme_ctrlr_probe(&trid, devhandle, dummy_probe_cb, cb_ctx); CU_ASSERT(rc == -1); /* happy path */ @@ -754,7 +754,7 @@ test_nvme_ctrlr_probe(void) MOCK_SET_P(nvme_transport_ctrlr_construct, struct spdk_nvme_ctrlr *, &ut_nvme_transport_ctrlr_construct); TAILQ_INIT(&g_nvme_init_ctrlrs); - rc = nvme_ctrlr_probe(trid, devhandle, dummy_probe_cb, cb_ctx); + rc = nvme_ctrlr_probe(&trid, devhandle, dummy_probe_cb, cb_ctx); CU_ASSERT(rc == 0); dummy = TAILQ_FIRST(&g_nvme_init_ctrlrs); CU_ASSERT(dummy == &ut_nvme_transport_ctrlr_construct);