diff --git a/lib/nvme/nvme.c b/lib/nvme/nvme.c index d8b88cef5..294f840fd 100644 --- a/lib/nvme/nvme.c +++ b/lib/nvme/nvme.c @@ -106,7 +106,8 @@ nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl) * Poll qpair for completions until a command completes. * * \param qpair queue to poll - * \param status completion status + * \param status completion status. The user must fill this structure with zeroes before calling + * this function * \param robust_mutex optional robust mutex to lock while polling qpair * * \return 0 if command completed without error, @@ -122,7 +123,6 @@ spdk_nvme_wait_for_completion_robust_lock( struct nvme_completion_poll_status *status, pthread_mutex_t *robust_mutex) { - memset(status, 0, sizeof(*status)); int rc; while (status->done == false) { @@ -160,7 +160,8 @@ spdk_nvme_wait_for_completion(struct spdk_nvme_qpair *qpair, * Poll qpair for completions until a command completes. * * \param qpair queue to poll - * \param status completion status + * \param status completion status. The user must fill this structure with zeroes before calling + * this function * \param timeout_in_secs optional timeout * * \return 0 if command completed without error, @@ -178,7 +179,6 @@ spdk_nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair, uint64_t timeout_tsc = 0; int rc = 0; - memset(status, 0, sizeof(*status)); if (timeout_in_secs) { timeout_tsc = spdk_get_ticks() + timeout_in_secs * spdk_get_ticks_hz(); } diff --git a/lib/nvme/nvme_ctrlr.c b/lib/nvme/nvme_ctrlr.c index 33900faed..a8d6255d9 100644 --- a/lib/nvme/nvme_ctrlr.c +++ b/lib/nvme/nvme_ctrlr.c @@ -593,7 +593,7 @@ static int nvme_ctrlr_set_intel_support_log_pages(struct spdk_nvme_ctrlr *ctrlr) return -ENXIO; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); spdk_free(log_page_directory); @@ -673,7 +673,7 @@ nvme_ctrlr_set_arbitration_feature(struct spdk_nvme_ctrlr *ctrlr) return; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return; @@ -1362,7 +1362,7 @@ nvme_ctrlr_identify_active_ns(struct spdk_nvme_ctrlr *ctrlr) return -ENOMEM; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); spdk_free(new_ns_list); @@ -1375,6 +1375,7 @@ nvme_ctrlr_identify_active_ns(struct spdk_nvme_ctrlr *ctrlr) * there are no more active namespaces */ for (i = 0; i < num_pages; i++) { + memset(status, 0, sizeof(*status)); rc = nvme_ctrlr_cmd_identify(ctrlr, SPDK_NVME_IDENTIFY_ACTIVE_NS_LIST, 0, next_nsid, &new_ns_list[1024 * i], sizeof(struct spdk_nvme_ns_list), nvme_completion_poll_cb, status); @@ -2961,7 +2962,7 @@ spdk_nvme_ctrlr_attach_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, int res; struct spdk_nvme_ns *ns; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -2999,7 +3000,7 @@ spdk_nvme_ctrlr_detach_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, int res; struct spdk_nvme_ns *ns; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -3040,7 +3041,7 @@ spdk_nvme_ctrlr_create_ns(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ns_dat uint32_t nsid; struct spdk_nvme_ns *ns; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return 0; @@ -3079,7 +3080,7 @@ spdk_nvme_ctrlr_delete_ns(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid) int res; struct spdk_nvme_ns *ns; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -3117,7 +3118,7 @@ spdk_nvme_ctrlr_format(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, struct nvme_completion_poll_status *status; int res; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -3171,7 +3172,7 @@ spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, ui return -1; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -3185,6 +3186,7 @@ spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, ui while (size_remaining > 0) { transfer = spdk_min(size_remaining, ctrlr->min_page_size); + memset(status, 0, sizeof(*status)); res = nvme_ctrlr_cmd_fw_image_download(ctrlr, transfer, offset, p, nvme_completion_poll_cb, status); @@ -3210,6 +3212,7 @@ spdk_nvme_ctrlr_update_firmware(struct spdk_nvme_ctrlr *ctrlr, void *payload, ui fw_commit.fs = slot; fw_commit.ca = commit_action; + memset(status, 0, sizeof(*status)); res = nvme_ctrlr_cmd_fw_commit(ctrlr, &fw_commit, nvme_completion_poll_cb, status); if (res) { @@ -3283,7 +3286,7 @@ spdk_nvme_ctrlr_security_receive(struct spdk_nvme_ctrlr *ctrlr, uint8_t secp, struct nvme_completion_poll_status *status; int res; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -3314,7 +3317,7 @@ spdk_nvme_ctrlr_security_send(struct spdk_nvme_ctrlr *ctrlr, uint8_t secp, struct nvme_completion_poll_status *status; int res; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; diff --git a/lib/nvme/nvme_fabric.c b/lib/nvme/nvme_fabric.c index 0ec30ba5c..d547dc714 100644 --- a/lib/nvme/nvme_fabric.c +++ b/lib/nvme/nvme_fabric.c @@ -50,7 +50,7 @@ nvme_fabric_prop_set_cmd(struct spdk_nvme_ctrlr *ctrlr, assert(size == SPDK_NVMF_PROP_SIZE_4 || size == SPDK_NVMF_PROP_SIZE_8); - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -93,7 +93,7 @@ nvme_fabric_prop_get_cmd(struct spdk_nvme_ctrlr *ctrlr, assert(size == SPDK_NVMF_PROP_SIZE_4 || size == SPDK_NVMF_PROP_SIZE_8); - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -231,7 +231,7 @@ nvme_fabric_get_discovery_log_page(struct spdk_nvme_ctrlr *ctrlr, struct nvme_completion_poll_status *status; int rc; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -294,7 +294,7 @@ nvme_fabric_ctrlr_scan(struct spdk_nvme_probe_ctx *probe_ctx, return -1; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); nvme_ctrlr_destruct(discovery_ctrlr); @@ -413,7 +413,7 @@ nvme_fabric_qpair_connect(struct spdk_nvme_qpair *qpair, uint32_t num_entries) return -ENOMEM; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); spdk_free(nvmf_data); diff --git a/lib/nvme/nvme_ns.c b/lib/nvme/nvme_ns.c index aea8662f0..183929e4a 100644 --- a/lib/nvme/nvme_ns.c +++ b/lib/nvme/nvme_ns.c @@ -117,7 +117,7 @@ nvme_ctrlr_identify_ns(struct spdk_nvme_ns *ns) struct spdk_nvme_ns_data *nsdata; int rc; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -163,7 +163,7 @@ nvme_ctrlr_identify_id_desc(struct spdk_nvme_ns *ns) return 0; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; diff --git a/lib/nvme/nvme_pcie.c b/lib/nvme/nvme_pcie.c index 0141caa81..3a1a7e272 100644 --- a/lib/nvme/nvme_pcie.c +++ b/lib/nvme/nvme_pcie.c @@ -1567,7 +1567,7 @@ _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme struct nvme_completion_poll_status *status; int rc; - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -1587,6 +1587,7 @@ _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme return -1; } + memset(status, 0, sizeof(*status)); rc = nvme_pcie_ctrlr_cmd_create_io_sq(qpair->ctrlr, qpair, nvme_completion_poll_cb, status); if (rc != 0) { free(status); @@ -1598,13 +1599,14 @@ _nvme_pcie_ctrlr_create_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme if (status->timed_out) { /* Request is still queued, the memory will be freed in a completion callback. allocate a new request */ - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; } } + memset(status, 0, sizeof(*status)); /* Attempt to delete the completion queue */ rc = nvme_pcie_ctrlr_cmd_delete_io_cq(qpair->ctrlr, qpair, nvme_completion_poll_cb, status); if (rc != 0) { @@ -1704,7 +1706,7 @@ nvme_pcie_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ goto free; } - status = malloc(sizeof(*status)); + status = calloc(1, sizeof(*status)); if (!status) { SPDK_ERRLOG("Failed to allocate status tracker\n"); return -ENOMEM; @@ -1724,6 +1726,7 @@ nvme_pcie_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ return -1; } + memset(status, 0, sizeof(*status)); /* Delete the completion queue */ rc = nvme_pcie_ctrlr_cmd_delete_io_cq(ctrlr, qpair, nvme_completion_poll_cb, status); if (rc != 0) { diff --git a/test/unit/lib/nvme/nvme.c/nvme_ut.c b/test/unit/lib/nvme/nvme.c/nvme_ut.c index bb619393b..9624adaf3 100644 --- a/test/unit/lib/nvme/nvme.c/nvme_ut.c +++ b/test/unit/lib/nvme/nvme.c/nvme_ut.c @@ -1229,18 +1229,18 @@ test_nvme_wait_for_completion(void) int rc = 0; memset(&qpair, 0, sizeof(qpair)); - memset(&g_status, 0, sizeof(g_status)); /* completion timeout */ + memset(&g_status, 0, sizeof(g_status)); completion_delay = 2; timeout_in_secs = 1; - g_status.done = true; rc = spdk_nvme_wait_for_completion_timeout(&qpair, &g_status, timeout_in_secs); CU_ASSERT(g_status.timed_out == true); CU_ASSERT(g_status.done == false); CU_ASSERT(rc == -ECANCELED); /* spdk_nvme_qpair_process_completions returns error */ + memset(&g_status, 0, sizeof(g_status)); g_process_comp_result = -1; completion_delay = 1; timeout_in_secs = 2; @@ -1254,6 +1254,7 @@ test_nvme_wait_for_completion(void) g_process_comp_result = 0; /* complete in time */ + memset(&g_status, 0, sizeof(g_status)); completion_delay = 1; timeout_in_secs = 2; rc = spdk_nvme_wait_for_completion_timeout(&qpair, &g_status, timeout_in_secs); @@ -1263,6 +1264,7 @@ test_nvme_wait_for_completion(void) /* spdk_nvme_wait_for_completion */ /* spdk_nvme_qpair_process_completions returns error */ + memset(&g_status, 0, sizeof(g_status)); g_process_comp_result = -1; rc = spdk_nvme_wait_for_completion(&qpair, &g_status); CU_ASSERT(rc == -ECANCELED); @@ -1271,9 +1273,9 @@ test_nvme_wait_for_completion(void) CU_ASSERT(g_status.cpl.status.sct == SPDK_NVME_SCT_GENERIC); CU_ASSERT(g_status.cpl.status.sc == SPDK_NVME_SC_ABORTED_SQ_DELETION); - g_process_comp_result = 0; - /* successful completion */ + memset(&g_status, 0, sizeof(g_status)); + g_process_comp_result = 0; rc = spdk_nvme_wait_for_completion(&qpair, &g_status); CU_ASSERT(rc == 0); CU_ASSERT(g_status.timed_out == false); diff --git a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c index 349d7ee37..e87387168 100644 --- a/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c +++ b/test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c @@ -275,6 +275,10 @@ void nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl) { struct nvme_completion_poll_status *status = arg; + /* This should not happen it test env since this callback is always called + * before wait_for_completion_* while this field can only be set to true in + * wait_for_completion_* functions */ + CU_ASSERT(status->timed_out == false); status->cpl = *cpl; status->done = true; @@ -288,7 +292,6 @@ spdk_nvme_wait_for_completion_robust_lock( struct nvme_completion_poll_status *status, pthread_mutex_t *robust_mutex) { - memset(status, 0, sizeof(*status)); if (spdk_nvme_qpair_process_completions(qpair, 0) < 0) { g_failed_status = status; status->timed_out = true;