From 524c16c6cce1992c8e63e9e2a645607ecf3a0414 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 3 Nov 2017 14:36:55 -0700 Subject: [PATCH 1/5] modules/kvs: Refactor cache_entry_get_raw() Function now takes const void ** isntead of just void **. Adjust callers appropriately. --- src/modules/kvs/cache.c | 3 ++- src/modules/kvs/cache.h | 3 ++- src/modules/kvs/commit.c | 2 +- src/modules/kvs/kvs.c | 8 ++++---- src/modules/kvs/lookup.c | 4 ++-- src/modules/kvs/test/cache.c | 12 +++++++----- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/modules/kvs/cache.c b/src/modules/kvs/cache.c index d9f4f5c82c81..d1735113905a 100644 --- a/src/modules/kvs/cache.c +++ b/src/modules/kvs/cache.c @@ -137,7 +137,8 @@ int cache_entry_force_clear_dirty (struct cache_entry *hp) return -1; } -int cache_entry_get_raw (struct cache_entry *hp, void **data, int *len) +int cache_entry_get_raw (struct cache_entry *hp, const void **data, + int *len) { if (!hp || !hp->valid) return -1; diff --git a/src/modules/kvs/cache.h b/src/modules/kvs/cache.h index c1b29d13572b..990b6c5babbc 100644 --- a/src/modules/kvs/cache.h +++ b/src/modules/kvs/cache.h @@ -79,7 +79,8 @@ int cache_entry_force_clear_dirty (struct cache_entry *hp); * cache_entry_set_raw() & cache_entry_set_treeobj() & * cache_entry_clear_data() returns -1 on error, 0 on success */ -int cache_entry_get_raw (struct cache_entry *hp, void **data, int *len); +int cache_entry_get_raw (struct cache_entry *hp, const void **data, + int *len); int cache_entry_set_raw (struct cache_entry *hp, void *data, int len); const json_t *cache_entry_get_treeobj (struct cache_entry *hp); diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index eb121c09974e..a1c8b4120298 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -166,7 +166,7 @@ void commit_cleanup_dirty_cache_entry (commit_t *c, struct cache_entry *hp) if (c->state == COMMIT_STATE_STORE || c->state == COMMIT_STATE_PRE_FINISHED) { href_t ref; - void *data; + const void *data; int len; int ret; diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 70c4900d4eec..1888bb0f9fda 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -364,8 +364,8 @@ static void content_store_completion (flux_future_t *f, void *arg) (void)content_store_get (f, arg); } -static int content_store_request_send (kvs_ctx_t *ctx, void *data, int len, - bool now) +static int content_store_request_send (kvs_ctx_t *ctx, const void *data, + int len, bool now) { flux_future_t *f; int saved_errno, rc = -1; @@ -424,7 +424,7 @@ static int commit_load_cb (commit_t *c, const char *ref, void *data) static int commit_cache_cb (commit_t *c, struct cache_entry *hp, void *data) { struct kvs_cb_data *cbd = data; - void *storedata; + const void *storedata; int storedatalen = 0; assert (cache_entry_get_dirty (hp)); @@ -1638,7 +1638,7 @@ static int store_initial_rootdir (kvs_ctx_t *ctx, json_t *o, href_t ref) cache_insert (ctx->cache, ref, hp); } if (!cache_entry_get_valid (hp)) { - void *data; + const void *data; int len; assert (o); if (cache_entry_set_treeobj (hp, o) < 0) { diff --git a/src/modules/kvs/lookup.c b/src/modules/kvs/lookup.c index 2a97415ffbfb..c2302249e73b 100644 --- a/src/modules/kvs/lookup.c +++ b/src/modules/kvs/lookup.c @@ -636,7 +636,7 @@ static int get_single_blobref_valref_value (lookup_t *lh, bool *stall) { struct cache_entry *hp; const char *reftmp; - void *valdata; + const void *valdata; int len; if (!(reftmp = treeobj_get_blobref (lh->wdirent, 0))) { @@ -707,7 +707,7 @@ static char *get_multi_blobref_valref_data (lookup_t *lh, int refcount, { struct cache_entry *hp; const char *reftmp; - void *valdata; + const void *valdata; int len; char *valbuf = NULL; int pos = 0; diff --git a/src/modules/kvs/test/cache.c b/src/modules/kvs/test/cache.c index 8c55671c0a41..fbf371a11a9c 100644 --- a/src/modules/kvs/test/cache.c +++ b/src/modules/kvs/test/cache.c @@ -84,7 +84,8 @@ void cache_entry_raw_tests (void) { struct cache_entry *e; json_t *o1; - char *data, *data2, *datatmp; + char *data, *data2; + const char *datatmp; int len; /* test empty cache entry later filled with raw data. @@ -122,7 +123,7 @@ void cache_entry_raw_tests (void) "cache_entry_set_treeobj, silent success"); o1 = NULL; - ok (cache_entry_get_raw (e, (void **)&datatmp, &len) == 0, + ok (cache_entry_get_raw (e, (const void **)&datatmp, &len) == 0, "raw data retrieved from cache entry"); ok (datatmp && strcmp (datatmp, data) == 0, "raw data matches expected string"); @@ -176,7 +177,7 @@ void cache_entry_raw_tests (void) json_decref (o1); o1 = NULL; - ok (cache_entry_get_raw (e, (void **)&datatmp, &len) == 0, + ok (cache_entry_get_raw (e, (const void **)&datatmp, &len) == 0, "raw data retrieved from cache entry"); ok (datatmp == NULL, "raw data is NULL"); @@ -274,7 +275,8 @@ void cache_entry_raw_and_treeobj_tests (void) struct cache_entry *e; json_t *o1, *otest; const json_t *otmp; - char *data, *datatmp; + char *data; + const char *datatmp; int len; /* test cache entry filled with raw data that is not valid treeobj @@ -329,7 +331,7 @@ void cache_entry_raw_and_treeobj_tests (void) "cache_entry_create works"); ok (cache_entry_set_treeobj (e, o1) == 0, "cache_entry_set_treeobj success"); - ok (cache_entry_get_raw (e, (void **)&datatmp, &len) == 0, + ok (cache_entry_get_raw (e, (const void **)&datatmp, &len) == 0, "cache_entry_get_raw returns success for get treeobj raw data"); ok (datatmp && strcmp (datatmp, data) == 0, "raw data matches expected string version of treeobj"); From 3377cc510a96f9b26659ebc5d1af5cab9bce688a Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 3 Nov 2017 15:07:30 -0700 Subject: [PATCH 2/5] common/libkvs/treeobj: Add treeobj_hash() Add new function to hash tree objects. Add unit tests appropriately. --- src/common/libkvs/test/treeobj.c | 28 ++++++++++++++++++++++++++++ src/common/libkvs/treeobj.c | 26 ++++++++++++++++++++++++++ src/common/libkvs/treeobj.h | 5 +++++ 3 files changed, 59 insertions(+) diff --git a/src/common/libkvs/test/treeobj.c b/src/common/libkvs/test/treeobj.c index 51cde252b11d..61c8a203bde3 100644 --- a/src/common/libkvs/test/treeobj.c +++ b/src/common/libkvs/test/treeobj.c @@ -742,6 +742,33 @@ void test_corner_cases (void) json_decref (symlink); } +void test_hash (void) +{ + char buf[128]; + json_t *o; + + ok (treeobj_hash (NULL, NULL, NULL, -1) < 0 + && errno == EINVAL, + "treeobj_hash fails with EINVAL on bad input"); + + o = json_object (); + ok (treeobj_hash ("sha1", o, buf, 128) < 0 + && errno == EINVAL, + "treeobj_hash fails with EINVAL on non-treeobj"); + json_decref (o); + + o = treeobj_create_val ("foo", 3); + + ok (treeobj_hash ("sha1", o, buf, 1) < 0 + && errno == EINVAL, + "treeobj_hash fails with EINVAL on too small buffer"); + + ok (treeobj_hash ("sha1", o, buf, 128) == 0, + "treeobj_hash success"); + + json_decref (o); +} + int main(int argc, char** argv) { plan (NO_PLAN); @@ -757,6 +784,7 @@ int main(int argc, char** argv) test_corner_cases (); test_codec (); + test_hash (); done_testing(); } diff --git a/src/common/libkvs/treeobj.c b/src/common/libkvs/treeobj.c index dd3635a85b00..38c4947c4835 100644 --- a/src/common/libkvs/treeobj.c +++ b/src/common/libkvs/treeobj.c @@ -569,6 +569,32 @@ char *treeobj_encode (const json_t *obj) return json_dumps (obj, JSON_COMPACT|JSON_SORT_KEYS); } +int treeobj_hash (const char *hash_name, json_t *obj, char *s, int size) +{ + char *tmp = NULL; + int rc = -1; + + if (!hash_name || !obj || !s || size <= 0) { + errno = EINVAL; + goto error; + } + + if (treeobj_validate (obj) < 0) + goto error; + + if (!(tmp = treeobj_encode (obj))) + goto error; + + if (blobref_hash (hash_name, (uint8_t *)tmp, strlen (tmp), + s, size) < 0) + goto error; + + rc = 0; + error: + free (tmp); + return rc; +} + /* * vi:tabstop=4 shiftwidth=4 expandtab */ diff --git a/src/common/libkvs/treeobj.h b/src/common/libkvs/treeobj.h index b987b0a47028..3243280702bc 100644 --- a/src/common/libkvs/treeobj.h +++ b/src/common/libkvs/treeobj.h @@ -114,6 +114,11 @@ json_t *treeobj_decode (const char *buf); json_t *treeobj_decodeb (const char *buf, size_t buflen); char *treeobj_encode (const json_t *obj); +/* Calculate hash of a treeobj + * Returns 0 on success, -1 on error with errno set + */ +int treeobj_hash (const char *hash_name, json_t *obj, char *s, int size); + #endif /* !_FLUX_KVS_TREEOBJ_H */ /* From 4b1eaccddba216ef0ed0af6cb2266ebd8e022b6b Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 3 Nov 2017 15:23:42 -0700 Subject: [PATCH 3/5] modules/kvs: Remove kvs_util_json_hash() Replace with call to treeobj_hash(). --- src/modules/kvs/commit.c | 4 +- src/modules/kvs/kvs.c | 4 +- src/modules/kvs/kvs_util.c | 19 +----- src/modules/kvs/kvs_util.h | 9 +-- src/modules/kvs/test/commit.c | 104 ++++++++++++++++---------------- src/modules/kvs/test/kvs_util.c | 14 ----- src/modules/kvs/test/lookup.c | 34 +++++------ 7 files changed, 75 insertions(+), 113 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index a1c8b4120298..974d0ba62d50 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -236,9 +236,9 @@ static int store_cache (commit_t *c, int current_epoch, json_t *o, blobref_hash (c->cm->hash_name, data, len, ref, sizeof (href_t)); } else { - if (kvs_util_json_hash (c->cm->hash_name, o, ref) < 0) { + if (treeobj_hash (c->cm->hash_name, o, ref, sizeof (href_t)) < 0) { saved_errno = errno; - flux_log_error (c->cm->h, "kvs_util_json_hash"); + flux_log_error (c->cm->h, "treeobj_hash"); goto done; } } diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 1888bb0f9fda..80f2ff62370d 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -1622,9 +1622,9 @@ static int store_initial_rootdir (kvs_ctx_t *ctx, json_t *o, href_t ref) int rc = -1; int saved_errno, ret; - if (kvs_util_json_hash (ctx->hash_name, o, ref) < 0) { + if (treeobj_hash (ctx->hash_name, o, ref, sizeof (href_t)) < 0) { saved_errno = errno; - flux_log_error (ctx->h, "%s: kvs_util_json_hash", + flux_log_error (ctx->h, "%s: treeobj_hash", __FUNCTION__); goto decref_done; } diff --git a/src/modules/kvs/kvs_util.c b/src/modules/kvs/kvs_util.c index 52676b190f11..b24f170a4d69 100644 --- a/src/modules/kvs/kvs_util.c +++ b/src/modules/kvs/kvs_util.c @@ -42,8 +42,7 @@ char *kvs_util_json_dumps (json_t *o) { /* Must pass JSON_ENCODE_ANY, can be called on any object. Must * set JSON_SORT_KEYS, two different objects with different - * internal order should map to same string (and reference when - * used by kvs_util_json_hash()). + * internal order should map to same string. */ int flags = JSON_ENCODE_ANY | JSON_COMPACT | JSON_SORT_KEYS; char *s; @@ -74,22 +73,6 @@ int kvs_util_json_encoded_size (json_t *o, size_t *size) return 0; } -int kvs_util_json_hash (const char *hash_name, json_t *o, href_t ref) -{ - char *s = NULL; - int rc = -1; - - if (!(s = kvs_util_json_dumps (o))) - goto error; - if (blobref_hash (hash_name, (uint8_t *)s, strlen (s), - ref, sizeof (href_t)) < 0) - goto error; - rc = 0; -error: - free (s); - return rc; -} - char *kvs_util_normalize_key (const char *key, bool *want_directory) { const char sep = '.'; diff --git a/src/modules/kvs/kvs_util.h b/src/modules/kvs/kvs_util.h index fce0264e0ed3..e1772655948d 100644 --- a/src/modules/kvs/kvs_util.h +++ b/src/modules/kvs/kvs_util.h @@ -7,8 +7,7 @@ #include "types.h" /* Get compact string representation of json object, or json null - * object if o is NULL. Use this function for consistency, especially - * when dealing with data that may be hashed via kvs_util_json_hash(). + * object if o is NULL. Use this function for consistency. * * Returns NULL on error */ @@ -17,12 +16,6 @@ char *kvs_util_json_dumps (json_t *o); /* returns 0 on success, -1 on failure */ int kvs_util_json_encoded_size (json_t *o, size_t *size); -/* Calculate hash of a json object - * - * Returns -1 on error, 0 on success - */ -int kvs_util_json_hash (const char *hash_name, json_t *o, href_t ref); - /* Normalize a KVS key * Returns new key string (caller must free), or NULL with errno set. * On success, 'want_directory' is set to true if key had a trailing diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index dc0b97ee7ab4..8f785e25894d 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -80,8 +80,8 @@ struct cache *create_cache_with_empty_rootdir (href_t ref) ok ((cache = cache_create ()) != NULL, "cache_create works"); - ok (kvs_util_json_hash ("sha1", rootdir, ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", rootdir, ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); ok ((hp = create_cache_entry_treeobj (rootdir)) != NULL, "create_cache_entry_treeobj works"); cache_insert (cache, ref, hp); @@ -631,8 +631,8 @@ void commit_basic_root_not_dir (void) /* make a non-dir root */ root = treeobj_create_val ("abcd", 4); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -699,8 +699,8 @@ void commit_process_root_missing (void) ok ((rootdir = treeobj_create_dir ()) != NULL, "treeobj_create_dir works"); - ok (kvs_util_json_hash ("sha1", rootdir, rootref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", rootdir, rootref, sizeof (href_t)) == 0, + "treeobj_hash worked"); json_decref (rootdir); @@ -797,16 +797,16 @@ void commit_process_missing_ref (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); /* don't add dir entry, we want it to miss */ root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -896,16 +896,16 @@ void commit_process_error_callbacks (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); /* don't add dir entry, we want it to miss */ root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -985,16 +985,16 @@ void commit_process_error_callbacks_partway (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, dir_ref, create_cache_entry_treeobj (dir)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1042,8 +1042,8 @@ void commit_process_invalid_operation (void) /* This root is an empty root */ root = treeobj_create_dir (); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1137,8 +1137,8 @@ void commit_process_invalid_hash (void) /* This root is an empty root */ root = treeobj_create_dir (); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1192,8 +1192,8 @@ void commit_process_follow_link (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, dir_ref, create_cache_entry_treeobj (dir)); @@ -1201,8 +1201,8 @@ void commit_process_follow_link (void) treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); treeobj_insert_entry (root, "symlink", treeobj_create_symlink ("dir")); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1259,8 +1259,8 @@ void commit_process_dirval_test (void) root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", dir); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1317,16 +1317,16 @@ void commit_process_delete_test (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, dir_ref, create_cache_entry_treeobj (dir)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1372,8 +1372,8 @@ void commit_process_delete_nosubdir_test (void) /* This root is an empty root */ root = treeobj_create_dir (); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1426,16 +1426,16 @@ void commit_process_delete_filevalinpath_test (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, dir_ref, create_cache_entry_treeobj (dir)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1488,8 +1488,8 @@ void commit_process_bad_dirrefs (void) dir = treeobj_create_dir (); treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, dir_ref, create_cache_entry_treeobj (dir)); @@ -1499,8 +1499,8 @@ void commit_process_bad_dirrefs (void) root = treeobj_create_dir (); treeobj_insert_entry (root, "dir", dirref); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1570,8 +1570,8 @@ void commit_process_big_fileval (void) root = treeobj_create_dir (); treeobj_insert_entry (root, "val", treeobj_create_val ("42", 2)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1716,16 +1716,16 @@ void commit_process_giant_dir (void) treeobj_insert_entry (dir, "val0e00", treeobj_create_val ("E", 1)); treeobj_insert_entry (dir, "valF000", treeobj_create_val ("f", 1)); - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", dir, dir_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, dir_ref, create_cache_entry_treeobj (dir)); root = treeobj_create_dir (); treeobj_insert_entry (dir, "dir", treeobj_create_dirref (dir_ref)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1801,8 +1801,8 @@ void commit_process_append (void) treeobj_insert_entry (root, "val", treeobj_create_val ("abcd", 4)); treeobj_insert_entry (root, "valref", treeobj_create_val ("ABCD", 4)); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); @@ -1927,8 +1927,8 @@ void commit_process_append_errors (void) treeobj_insert_entry (root, "dir", treeobj_create_dir ()); treeobj_insert_entry (root, "symlink", treeobj_create_symlink ("dir")); - ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, - "kvs_util_json_hash worked"); + ok (treeobj_hash ("sha1", root, root_ref, sizeof (href_t)) == 0, + "treeobj_hash worked"); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); diff --git a/src/modules/kvs/test/kvs_util.c b/src/modules/kvs/test/kvs_util.c index 09c89e41a484..b78a96b6cba2 100644 --- a/src/modules/kvs/test/kvs_util.c +++ b/src/modules/kvs/test/kvs_util.c @@ -63,25 +63,11 @@ void test_norm (void) int main (int argc, char *argv[]) { json_t *obj; - href_t ref; char *s1, *s2; size_t size; plan (NO_PLAN); - obj = json_object (); - json_object_set_new (obj, "A", json_string ("foo")); - json_object_set_new (obj, "B", json_string ("bar")); - json_object_set_new (obj, "C", json_string ("cow")); - - ok (kvs_util_json_hash ("sha1", obj, ref) == 0, - "kvs_util_json_hash works on sha1"); - - ok (kvs_util_json_hash ("foobar", obj, ref) < 0, - "kvs_util_json_hash error on bad hash name"); - - json_decref (obj); - obj = json_object (); json_object_set_new (obj, "A", json_string ("a")); json_object_set_new (obj, "B", json_string ("b")); diff --git a/src/modules/kvs/test/lookup.c b/src/modules/kvs/test/lookup.c index 081163f12a6a..34a423215c17 100644 --- a/src/modules/kvs/test/lookup.c +++ b/src/modules/kvs/test/lookup.c @@ -412,7 +412,7 @@ void lookup_root (void) { cache_insert (cache, valref_ref, create_cache_entry_raw (strdup ("abcd"), 4)); root = treeobj_create_dir (); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); /* flags = 0, should error EISDIR */ @@ -517,7 +517,7 @@ void lookup_basic (void) { dirref_test = treeobj_create_dir (); treeobj_insert_entry (dirref_test, "dummy", treeobj_create_val ("dummy", 5)); - kvs_util_json_hash ("sha1", dirref_test, dirref_test_ref); + treeobj_hash ("sha1", dirref_test, dirref_test_ref, sizeof (href_t)); cache_insert (cache, dirref_test_ref, create_cache_entry_treeobj (dirref_test)); dir = treeobj_create_dir (); @@ -540,13 +540,13 @@ void lookup_basic (void) { treeobj_insert_entry (dirref, "valref_multi_with_dirref", valref_multi_with_dirref); - kvs_util_json_hash ("sha1", dirref, dirref_ref); + treeobj_hash ("sha1", dirref, dirref_ref, sizeof (href_t)); cache_insert (cache, dirref_ref, create_cache_entry_treeobj (dirref)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dirref", treeobj_create_dirref (dirref_ref)); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); /* lookup dir via dirref */ @@ -774,7 +774,7 @@ void lookup_errors (void) { dirref = treeobj_create_dir (); treeobj_insert_entry (dirref, "val", treeobj_create_val ("bar", 3)); - kvs_util_json_hash ("sha1", dirref, dirref_ref); + treeobj_hash ("sha1", dirref, dirref_ref, sizeof (href_t)); cache_insert (cache, dirref_ref, create_cache_entry_treeobj (dirref)); dir = treeobj_create_dir (); @@ -799,7 +799,7 @@ void lookup_errors (void) { treeobj_insert_entry (root, "dirref_multi", dirref_multi); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); /* Lookup non-existent field. Not ENOENT - caller of lookup @@ -1083,7 +1083,7 @@ void lookup_links (void) { dirref3 = treeobj_create_dir (); treeobj_insert_entry (dirref3, "val", treeobj_create_val ("baz", 3)); - kvs_util_json_hash ("sha1", dirref3, dirref3_ref); + treeobj_hash ("sha1", dirref3, dirref3_ref, sizeof (href_t)); cache_insert (cache, dirref3_ref, create_cache_entry_treeobj (dirref3)); dir = treeobj_create_dir (); @@ -1095,7 +1095,7 @@ void lookup_links (void) { treeobj_insert_entry (dirref2, "dir", dir); treeobj_insert_entry (dirref2, "dirref", treeobj_create_dirref (dirref3_ref)); treeobj_insert_entry (dirref2, "symlink", treeobj_create_symlink ("dirref2.val")); - kvs_util_json_hash ("sha1", dirref2, dirref2_ref); + treeobj_hash ("sha1", dirref2, dirref2_ref, sizeof (href_t)); cache_insert (cache, dirref2_ref, create_cache_entry_treeobj (dirref2)); dirref1 = treeobj_create_dir (); @@ -1104,13 +1104,13 @@ void lookup_links (void) { treeobj_insert_entry (dirref1, "link2valref", treeobj_create_symlink ("dirref2.valref")); treeobj_insert_entry (dirref1, "link2dir", treeobj_create_symlink ("dirref2.dir")); treeobj_insert_entry (dirref1, "link2symlink", treeobj_create_symlink ("dirref2.symlink")); - kvs_util_json_hash ("sha1", dirref1, dirref1_ref); + treeobj_hash ("sha1", dirref1, dirref1_ref, sizeof (href_t)); cache_insert (cache, dirref1_ref, create_cache_entry_treeobj (dirref1)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dirref1", treeobj_create_dirref (dirref1_ref)); treeobj_insert_entry (root, "dirref2", treeobj_create_dirref (dirref2_ref)); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); /* lookup val, follow two links */ @@ -1280,18 +1280,18 @@ void lookup_alt_root (void) { dirref1 = treeobj_create_dir (); treeobj_insert_entry (dirref1, "val", treeobj_create_val ("foo", 3)); - kvs_util_json_hash ("sha1", dirref1, dirref1_ref); + treeobj_hash ("sha1", dirref1, dirref1_ref, sizeof (href_t)); cache_insert (cache, dirref1_ref, create_cache_entry_treeobj (dirref1)); dirref2 = treeobj_create_dir (); treeobj_insert_entry (dirref2, "val", treeobj_create_val ("bar", 3)); - kvs_util_json_hash ("sha1", dirref2, dirref2_ref); + treeobj_hash ("sha1", dirref2, dirref2_ref, sizeof (href_t)); cache_insert (cache, dirref2_ref, create_cache_entry_treeobj (dirref2)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dirref1", treeobj_create_dirref (dirref1_ref)); treeobj_insert_entry (root, "dirref2", treeobj_create_dirref (dirref2_ref)); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); cache_insert (cache, root_ref, create_cache_entry_treeobj (root)); /* lookup val, alt root-ref dirref1_ref */ @@ -1341,7 +1341,7 @@ void lookup_stall_root (void) { root = treeobj_create_dir (); treeobj_insert_entry (root, "val", treeobj_create_val ("foo", 3)); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); /* do not insert entries into cache until later for these stall tests */ @@ -1454,17 +1454,17 @@ void lookup_stall (void) { treeobj_append_blobref (valref_tmp, valrefmisc2_ref); treeobj_insert_entry (dirref1, "valrefmisc_multi", valref_tmp); - kvs_util_json_hash ("sha1", dirref1, dirref1_ref); + treeobj_hash ("sha1", dirref1, dirref1_ref, sizeof (href_t)); dirref2 = treeobj_create_dir (); treeobj_insert_entry (dirref2, "val", treeobj_create_val ("bar", 3)); - kvs_util_json_hash ("sha1", dirref2, dirref2_ref); + treeobj_hash ("sha1", dirref2, dirref2_ref, sizeof (href_t)); root = treeobj_create_dir (); treeobj_insert_entry (root, "dirref1", treeobj_create_dirref (dirref1_ref)); treeobj_insert_entry (root, "dirref2", treeobj_create_dirref (dirref2_ref)); treeobj_insert_entry (root, "symlink", treeobj_create_symlink ("dirref2")); - kvs_util_json_hash ("sha1", root, root_ref); + treeobj_hash ("sha1", root, root_ref, sizeof (href_t)); /* do not insert entries into cache until later for these stall tests */ From 036ed34732e6f8191901069f989b0f54ea2242cc Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 3 Nov 2017 16:01:23 -0700 Subject: [PATCH 4/5] modules/kvs: Cleanup code in commit_unroll() With RFC11 changes, we always know the data in a treeobj val object is a base64 encoded string. Therefore, we do not need to call kvs_util_json_encoded_size to get the encoded size. We just need to call strlen() on the string value. --- src/modules/kvs/commit.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index 974d0ba62d50..7c5dbeea306f 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -344,13 +344,14 @@ static int commit_unroll (commit_t *c, int current_epoch, json_t *dir) } else if (treeobj_is_val (dir_entry)) { json_t *val_data; - size_t size; + const char *str; if (!(val_data = treeobj_get_data (dir_entry))) return -1; - if (kvs_util_json_encoded_size (val_data, &size) < 0) - return -1; - if (size > BLOBREF_MAX_STRING_SIZE) { + /* jansson >= 2.7 could use json_string_length() instead */ + str = json_string_value (val_data); + assert (str); + if (strlen (str) > BLOBREF_MAX_STRING_SIZE) { if ((ret = store_cache (c, current_epoch, val_data, true, ref, &hp)) < 0) return -1; From bebc34769b94cb18e49658dd960ae36751d17e8c Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 3 Nov 2017 15:51:30 -0700 Subject: [PATCH 5/5] modules/kvs/kvs_util: Remove unused functions With recent changes, kvs_util_json_dumps() and kvs_util_json_encoded_size() are no longer used, so remove them and associated unit tests. Also remove now unnecessary header includes. --- src/modules/kvs/kvs_util.c | 45 +----------------- src/modules/kvs/kvs_util.h | 15 ------ src/modules/kvs/test/kvs_util.c | 83 ++------------------------------- 3 files changed, 5 insertions(+), 138 deletions(-) diff --git a/src/modules/kvs/kvs_util.c b/src/modules/kvs/kvs_util.c index b24f170a4d69..1683c7490022 100644 --- a/src/modules/kvs/kvs_util.c +++ b/src/modules/kvs/kvs_util.c @@ -27,51 +27,8 @@ #endif #include #include -#include -#include #include -#include -#include -#include - -#include "src/common/libutil/blobref.h" - -#include "types.h" - -char *kvs_util_json_dumps (json_t *o) -{ - /* Must pass JSON_ENCODE_ANY, can be called on any object. Must - * set JSON_SORT_KEYS, two different objects with different - * internal order should map to same string. - */ - int flags = JSON_ENCODE_ANY | JSON_COMPACT | JSON_SORT_KEYS; - char *s; - if (!o) { - if (!(s = strdup ("null"))) { - errno = ENOMEM; - return NULL; - } - return s; - } - if (!(s = json_dumps (o, flags))) { - errno = ENOMEM; - return NULL; - } - return s; -} - -int kvs_util_json_encoded_size (json_t *o, size_t *size) -{ - char *s = kvs_util_json_dumps (o); - if (!s) { - errno = ENOMEM; - return -1; - } - if (size) - *size = strlen (s); - free (s); - return 0; -} +#include char *kvs_util_normalize_key (const char *key, bool *want_directory) { diff --git a/src/modules/kvs/kvs_util.h b/src/modules/kvs/kvs_util.h index e1772655948d..9e3f74e573dd 100644 --- a/src/modules/kvs/kvs_util.h +++ b/src/modules/kvs/kvs_util.h @@ -1,20 +1,5 @@ #ifndef _FLUX_KVS_UTIL_H #define _FLUX_KVS_UTIL_H -#include - -#include "src/common/libutil/tstat.h" -#include "waitqueue.h" -#include "types.h" - -/* Get compact string representation of json object, or json null - * object if o is NULL. Use this function for consistency. - * - * Returns NULL on error - */ -char *kvs_util_json_dumps (json_t *o); - -/* returns 0 on success, -1 on failure */ -int kvs_util_json_encoded_size (json_t *o, size_t *size); /* Normalize a KVS key * Returns new key string (caller must free), or NULL with errno set. diff --git a/src/modules/kvs/test/kvs_util.c b/src/modules/kvs/test/kvs_util.c index b78a96b6cba2..f731ea7f6ac4 100644 --- a/src/modules/kvs/test/kvs_util.c +++ b/src/modules/kvs/test/kvs_util.c @@ -2,17 +2,18 @@ #include "config.h" #endif #include -#include +#include #include "src/common/libtap/tap.h" #include "src/modules/kvs/kvs_util.h" -#include "src/modules/kvs/types.h" -void test_norm (void) +int main (int argc, char *argv[]) { char *s; bool dirflag; + plan (NO_PLAN); + s = kvs_util_normalize_key ("a.b.c.d.e", &dirflag); ok (s != NULL && !strcmp (s, "a.b.c.d.e") && dirflag == false, "kvs_util_normalize_key works on normal key"); @@ -57,82 +58,6 @@ void test_norm (void) ok (s != NULL && !strcmp (s, "."), "kvs_util_normalize_key transforms several standalone separators to one"); free (s); -} - - -int main (int argc, char *argv[]) -{ - json_t *obj; - char *s1, *s2; - size_t size; - - plan (NO_PLAN); - - obj = json_object (); - json_object_set_new (obj, "A", json_string ("a")); - json_object_set_new (obj, "B", json_string ("b")); - json_object_set_new (obj, "C", json_string ("c")); - - ok ((s1 = kvs_util_json_dumps (obj)) != NULL, - "kvs_util_json_dumps works"); - - /* json object is sorted and compacted */ - s2 = "{\"A\":\"a\",\"B\":\"b\",\"C\":\"c\"}"; - - ok (!strcmp (s1, s2), - "kvs_util_json_dumps dumps correct string"); - - ok (kvs_util_json_encoded_size (obj, NULL) == 0, - "kvs_util_json_encoded_size works w/ NULL size param"); - - ok (kvs_util_json_encoded_size (obj, &size) == 0, - "kvs_util_json_encoded_size works"); - - ok (size == strlen (s2), - "kvs_util_json_encoded_size returns correct size"); - - free (s1); - s1 = NULL; - json_decref (obj); - - obj = json_null (); - - ok ((s1 = kvs_util_json_dumps (obj)) != NULL, - "kvs_util_json_dumps works"); - - s2 = "null"; - - ok (!strcmp (s1, s2), - "kvs_util_json_dumps works on null object"); - - ok (kvs_util_json_encoded_size (obj, &size) == 0, - "kvs_util_json_encoded_size works"); - - ok (size == strlen (s2), - "kvs_util_json_encoded_size returns correct size"); - - free (s1); - s1 = NULL; - json_decref (obj); - - ok ((s1 = kvs_util_json_dumps (NULL)) != NULL, - "kvs_util_json_dumps works on NULL pointer"); - - s2 = "null"; - - ok (!strcmp (s1, s2), - "kvs_util_json_dumps works on NULL pointer"); - - ok (kvs_util_json_encoded_size (NULL, &size) == 0, - "kvs_util_json_encoded_size works on NULL pointer"); - - ok (size == strlen (s2), - "kvs_util_json_encoded_size returns correct size"); - - free (s1); - s1 = NULL; - - test_norm (); done_testing (); return (0);