Skip to content

Commit

Permalink
DAOS-13907 kv: allow users to insert keys with empty values
Browse files Browse the repository at this point in the history 
- also fix some potential double free on error path

Required-githooks: true

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@intel.com>
  • Loading branch information
@mchaarawi
mchaarawi committed Jul 7, 2023
1 parent dabc47e commit 1ae6a76
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 18 deletions.
17 changes: 13 additions & 4 deletions src/client/kv/dc_kv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -403,6 +404,7 @@ dc_kv_put(tse_task_t *task)
rc = tse_task_register_comp_cb(task, free_io_params_cb, &params, 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)
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -493,6 +497,7 @@ dc_kv_get(tse_task_t *task)
rc = tse_task_register_comp_cb(task, free_io_params_cb, &params, 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)
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -553,6 +560,7 @@ dc_kv_remove(tse_task_t *task)
rc = tse_task_register_comp_cb(task, free_io_params_cb, &params, 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)
Expand All @@ -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;
Expand Down
44 changes: 30 additions & 14 deletions src/tests/suite/daos_kv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 1ae6a76

Please sign in to comment.