From 1ae6a7638c5f49a58ee6f0c462a61179530f7421 Mon Sep 17 00:00:00 2001 From: Mohamad Chaarawi Date: Fri, 7 Jul 2023 13:32:56 +0000 Subject: [PATCH] DAOS-13907 kv: allow users to insert keys with empty values - also fix some potential double free on error path Required-githooks: true Signed-off-by: Mohamad Chaarawi --- src/client/kv/dc_kv.c | 17 +++++++++++---- src/tests/suite/daos_kv.c | 44 ++++++++++++++++++++++++++------------- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/client/kv/dc_kv.c b/src/client/kv/dc_kv.c index 3924c48c04b..ff3d7a7ac2f 100644 --- a/src/client/kv/dc_kv.c +++ b/src/client/kv/dc_kv.c @@ -358,9 +358,10 @@ dc_kv_put(tse_task_t *task) daos_obj_update_t *update_args; tse_task_t *update_task; struct io_params *params = NULL; + bool free_params = true; int rc; - if (args->key == NULL || args->buf_size == 0 || args->buf == NULL) + if (args->key == NULL) D_GOTO(err_task, rc = -DER_INVAL); kv = kv_hdl2ptr(args->oh); @@ -403,6 +404,7 @@ dc_kv_put(tse_task_t *task) rc = tse_task_register_comp_cb(task, free_io_params_cb, ¶ms, sizeof(params)); if (rc != 0) D_GOTO(err_utask, rc); + free_params = false; rc = tse_task_register_deps(task, 1, &update_task); if (rc != 0) @@ -418,7 +420,8 @@ dc_kv_put(tse_task_t *task) tse_task_complete(update_task, rc); err_task: tse_task_complete(task, rc); - D_FREE(params); + if (free_params) + D_FREE(params); if (kv) kv_decref(kv); return rc; @@ -434,6 +437,7 @@ dc_kv_get(tse_task_t *task) struct io_params *params = NULL; void *buf; daos_size_t *buf_size; + bool free_params = true; int rc; if (args->key == NULL) @@ -493,6 +497,7 @@ dc_kv_get(tse_task_t *task) rc = tse_task_register_comp_cb(task, free_io_params_cb, ¶ms, sizeof(params)); if (rc != 0) D_GOTO(err_ftask, rc); + free_params = false; rc = tse_task_register_deps(task, 1, &fetch_task); if (rc != 0) @@ -508,7 +513,8 @@ dc_kv_get(tse_task_t *task) tse_task_complete(fetch_task, rc); err_task: tse_task_complete(task, rc); - D_FREE(params); + if (free_params) + D_FREE(params); if (kv) kv_decref(kv); return rc; @@ -522,6 +528,7 @@ dc_kv_remove(tse_task_t *task) daos_obj_punch_t *punch_args; tse_task_t *punch_task; struct io_params *params = NULL; + bool free_params = true; int rc; if (args->key == NULL) @@ -553,6 +560,7 @@ dc_kv_remove(tse_task_t *task) rc = tse_task_register_comp_cb(task, free_io_params_cb, ¶ms, sizeof(params)); if (rc != 0) D_GOTO(err_ptask, rc); + free_params = false; rc = tse_task_register_deps(task, 1, &punch_task); if (rc != 0) @@ -568,7 +576,8 @@ dc_kv_remove(tse_task_t *task) tse_task_complete(punch_task, rc); err_task: tse_task_complete(task, rc); - D_FREE(params); + if (free_params) + D_FREE(params); if (kv) kv_decref(kv); return rc; diff --git a/src/tests/suite/daos_kv.c b/src/tests/suite/daos_kv.c index 701c5cc2ede..788caf1e73c 100644 --- a/src/tests/suite/daos_kv.c +++ b/src/tests/suite/daos_kv.c @@ -287,43 +287,59 @@ kv_cond_ops(void **state) val_out = 5; size = sizeof(int); print_message("Conditional FETCH of non existent Key(should fail)\n"); - rc = daos_kv_get(oh, DAOS_TX_NONE, DAOS_COND_KEY_GET, "Key2", - &size, &val_out, NULL); + rc = daos_kv_get(oh, DAOS_TX_NONE, DAOS_COND_KEY_GET, "Key2", &size, &val_out, NULL); assert_rc_equal(rc, -DER_NONEXIST); assert_int_equal(val_out, 5); val = 1; print_message("Conditional UPDATE of non existent Key(should fail)\n"); - rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_UPDATE, "Key1", - sizeof(int), &val, NULL); + rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_UPDATE, "Key1", sizeof(int), &val, NULL); assert_rc_equal(rc, -DER_NONEXIST); print_message("Conditional INSERT of non existent Key\n"); - rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_INSERT, "Key1", - sizeof(int), &val, NULL); + rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_INSERT, "Key1", sizeof(int), &val, NULL); assert_rc_equal(rc, 0); val = 2; print_message("Conditional INSERT of existing Key (Should fail)\n"); - rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_INSERT, "Key1", - sizeof(int), &val, NULL); + rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_INSERT, "Key1", sizeof(int), &val, NULL); assert_rc_equal(rc, -DER_EXIST); size = sizeof(int); print_message("Conditional FETCH of existing Key\n"); - rc = daos_kv_get(oh, DAOS_TX_NONE, DAOS_COND_KEY_GET, "Key1", - &size, &val_out, NULL); + rc = daos_kv_get(oh, DAOS_TX_NONE, DAOS_COND_KEY_GET, "Key1", &size, &val_out, NULL); assert_rc_equal(rc, 0); assert_int_equal(val_out, 1); print_message("Conditional Remove non existing Key (should fail)\n"); - rc = daos_kv_remove(oh, DAOS_TX_NONE, DAOS_COND_KEY_REMOVE, "Key2", - NULL); + rc = daos_kv_remove(oh, DAOS_TX_NONE, DAOS_COND_KEY_REMOVE, "Key2", NULL); assert_rc_equal(rc, -DER_NONEXIST); print_message("Conditional Remove existing Key\n"); - rc = daos_kv_remove(oh, DAOS_TX_NONE, DAOS_COND_KEY_REMOVE, "Key1", - NULL); + rc = daos_kv_remove(oh, DAOS_TX_NONE, DAOS_COND_KEY_REMOVE, "Key1", NULL); + assert_rc_equal(rc, 0); + + print_message("Conditional INSERT of Key with no value\n"); + rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_INSERT, "Empty_Key", 0, NULL, NULL); + assert_rc_equal(rc, 0); + + print_message("Conditional INSERT of existing (but empty) Key (should fail)\n"); + rc = daos_kv_put(oh, DAOS_TX_NONE, DAOS_COND_KEY_INSERT, "Empty_Key", sizeof(int), &val, + NULL); + assert_rc_equal(rc, -DER_EXIST); + + size = sizeof(int); + print_message("Conditional FETCH of existing but empty Key\n"); + rc = daos_kv_get(oh, DAOS_TX_NONE, DAOS_COND_KEY_GET, "Empty_Key", &size, &val_out, NULL); + assert_rc_equal(rc, 0); + assert_int_equal(size, 0); + + print_message("Update the empty Key with a no value update\n"); + rc = daos_kv_put(oh, DAOS_TX_NONE, 0, "Empty_Key", 0, NULL, NULL); + assert_rc_equal(rc, 0); + + print_message("Conditional Remove existing but empty Key\n"); + rc = daos_kv_remove(oh, DAOS_TX_NONE, DAOS_COND_KEY_REMOVE, "Empty_Key", NULL); assert_rc_equal(rc, 0); print_message("Destroying KV\n");