Commit 3fa7c33a authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

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: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/408395


Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 0692768e

lib/nvme/nvme.c

+3 −5
Original line number Diff line number Diff line
@@ -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");

			}

			return -1;

		}

test/unit/lib/nvme/nvme.c/nvme_ut.c

+4 −4
Original line number Diff line number Diff line
@@ -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);