From bc0a6e2d19c298858db4eeef7e303804a82c7984 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 2 Nov 2017 11:58:07 -0700 Subject: [PATCH] modules/kvs: Refactor cache_entry_get_treeobj() Return const json_t * from function, as we do not want users to modify any json object stored in the cache. --- src/modules/kvs/cache.c | 2 +- src/modules/kvs/cache.h | 2 +- src/modules/kvs/commit.c | 9 +++++---- src/modules/kvs/kvs.c | 2 +- src/modules/kvs/lookup.c | 17 +++++++---------- src/modules/kvs/test/cache.c | 23 ++++++++++++++--------- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/modules/kvs/cache.c b/src/modules/kvs/cache.c index e2a43774bad7..d9f4f5c82c81 100644 --- a/src/modules/kvs/cache.c +++ b/src/modules/kvs/cache.c @@ -186,7 +186,7 @@ int cache_entry_set_raw (struct cache_entry *hp, void *data, int len) return -1; } -json_t *cache_entry_get_treeobj (struct cache_entry *hp) +const json_t *cache_entry_get_treeobj (struct cache_entry *hp) { if (!hp || !hp->valid || !hp->data) return NULL; diff --git a/src/modules/kvs/cache.h b/src/modules/kvs/cache.h index e83c1a59d8ed..c1b29d13572b 100644 --- a/src/modules/kvs/cache.h +++ b/src/modules/kvs/cache.h @@ -82,7 +82,7 @@ int cache_entry_force_clear_dirty (struct cache_entry *hp); int cache_entry_get_raw (struct cache_entry *hp, void **data, int *len); int cache_entry_set_raw (struct cache_entry *hp, void *data, int len); -json_t *cache_entry_get_treeobj (struct cache_entry *hp); +const json_t *cache_entry_get_treeobj (struct cache_entry *hp); int cache_entry_set_treeobj (struct cache_entry *hp, json_t *o); /* Arrange for message handler represented by 'wait' to be restarted diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index db8ff1323b44..eb121c09974e 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -559,6 +559,7 @@ static int commit_link_dirent (commit_t *c, int current_epoch, } else if (treeobj_is_dirref (dir_entry)) { struct cache_entry *hp; const char *ref; + const json_t *subdirtmp; int refcount; if ((refcount = treeobj_get_count (dir_entry)) < 0) { @@ -584,13 +585,13 @@ static int commit_link_dirent (commit_t *c, int current_epoch, goto success; /* stall */ } - if (!(subdir = cache_entry_get_treeobj (hp))) { + if (!(subdirtmp = cache_entry_get_treeobj (hp))) { saved_errno = ENOTRECOVERABLE; goto done; } /* do not corrupt store by modifying orig. */ - if (!(subdir = treeobj_copy (subdir))) { + if (!(subdir = treeobj_deep_copy (subdirtmp))) { saved_errno = errno; goto done; } @@ -699,7 +700,7 @@ commit_process_t commit_process (commit_t *c, /* Make a copy of the root directory. */ struct cache_entry *hp; - json_t *rootdir; + const json_t *rootdir; /* Caller didn't call commit_iter_missing_refs() */ if (zlist_first (c->missing_refs_list)) @@ -725,7 +726,7 @@ commit_process_t commit_process (commit_t *c, return COMMIT_PROCESS_ERROR; } - if (!(c->rootcpy = treeobj_copy (rootdir))) { + if (!(c->rootcpy = treeobj_deep_copy (rootdir))) { c->errnum = errno; return COMMIT_PROCESS_ERROR; } diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 4c09215100a2..70c4900d4eec 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -1421,7 +1421,7 @@ static void setroot_event_cb (flux_t *h, flux_msg_handler_t *w, static int setroot_event_send (kvs_ctx_t *ctx, json_t *names) { - json_t *root = NULL; + const json_t *root = NULL; json_t *nullobj = NULL; flux_msg_t *msg = NULL; int saved_errno, rc = -1; diff --git a/src/modules/kvs/lookup.c b/src/modules/kvs/lookup.c index 573f61b5518e..2a97415ffbfb 100644 --- a/src/modules/kvs/lookup.c +++ b/src/modules/kvs/lookup.c @@ -53,7 +53,7 @@ typedef struct { int depth; char *path_copy; /* for internal parsing, do not use */ - json_t *dirent; + const json_t *dirent; zlist_t *pathcomps; } walk_level_t; @@ -82,7 +82,7 @@ struct lookup { /* if valref_missing_refs is true, iterate on refs, else * return missing_ref string. */ - json_t *valref_missing_refs; + const json_t *valref_missing_refs; const char *missing_ref; int errnum; /* errnum if error */ @@ -91,7 +91,7 @@ struct lookup { /* API internal */ json_t *root_dirent; zlist_t *levels; - json_t *wdirent; /* result after walk() */ + const json_t *wdirent; /* result after walk() */ enum { LOOKUP_STATE_INIT, LOOKUP_STATE_CHECK_ROOT, @@ -219,7 +219,7 @@ static walk_level_t *walk_levels_push (lookup_t *lh, */ static bool walk (lookup_t *lh) { - json_t *dir; + const json_t *dir; walk_level_t *wl = NULL; char *pathcomp; @@ -299,7 +299,7 @@ static bool walk (lookup_t *lh) /* Get directory reference of path component from directory */ - if (!(wl->dirent = treeobj_get_entry (dir, pathcomp))) { + if (!(wl->dirent = treeobj_peek_entry (dir, pathcomp))) { /* if entry does not exist, not necessarily ENOENT error, * let caller decide. If error not ENOENT, return to * caller. */ @@ -311,16 +311,13 @@ static bool walk (lookup_t *lh) /* Resolve dirent if it is a link */ if (treeobj_is_symlink (wl->dirent)) { - json_t *link; const char *linkstr; - if (!(link = treeobj_get_data (wl->dirent))) { + if (!(linkstr = treeobj_get_symlink (wl->dirent))) { lh->errnum = errno; goto error; } - linkstr = json_string_value (link); - /* Follow link if in middle of path or if end of path, * flags say we can follow it */ @@ -779,7 +776,7 @@ static int get_multi_blobref_valref_value (lookup_t *lh, int refcount, bool lookup (lookup_t *lh) { - json_t *valtmp = NULL; + const json_t *valtmp = NULL; const char *reftmp; struct cache_entry *hp; int refcount; diff --git a/src/modules/kvs/test/cache.c b/src/modules/kvs/test/cache.c index d2b2c1bc26e3..8c55671c0a41 100644 --- a/src/modules/kvs/test/cache.c +++ b/src/modules/kvs/test/cache.c @@ -190,7 +190,8 @@ void cache_entry_raw_tests (void) void cache_entry_treeobj_tests (void) { struct cache_entry *e; - json_t *otmp, *o1, *o2, *otest; + json_t *o1, *o2, *otest; + const json_t *otmp; char *data; /* Play with one entry. @@ -215,7 +216,7 @@ void cache_entry_treeobj_tests (void) "cache_entry_set_dirty fails b/c entry non-valid"); ok (cache_entry_get_dirty (e) == false, "cache entry does not set dirty, b/c no data"); - ok ((otmp = cache_entry_get_treeobj (e)) == NULL, + ok (cache_entry_get_treeobj (e) == NULL, "cache_entry_get_treeobj returns NULL, no treeobj set"); ok (cache_entry_set_treeobj (e, o1) == 0, "cache_entry_set_treeobj success"); @@ -259,7 +260,8 @@ void cache_entry_treeobj_tests (void) ok ((otmp = cache_entry_get_treeobj (e)) != NULL, "treeobj retrieved from cache entry"); otest = treeobj_create_val ("foo", 3); - ok (json_equal (otmp, otest) == 1, + /* XXX - json_equal takes const in jansson > 2.10 */ + ok (json_equal ((json_t *)otmp, otest) == 1, "expected treeobj object returned"); json_decref (otest); @@ -270,7 +272,8 @@ void cache_entry_treeobj_tests (void) void cache_entry_raw_and_treeobj_tests (void) { struct cache_entry *e; - json_t *o1, *otmp, *otest; + json_t *o1, *otest; + const json_t *otmp; char *data, *datatmp; int len; @@ -283,7 +286,7 @@ void cache_entry_raw_and_treeobj_tests (void) "cache_entry_create works"); ok (cache_entry_set_raw (e, data, strlen (data) + 1) == 0, "cache_entry_set_raw success"); - ok ((otmp = cache_entry_get_treeobj (e)) == NULL, + ok (cache_entry_get_treeobj (e) == NULL, "cache_entry_get_treeobj returns NULL for non-treeobj raw data"); cache_entry_destroy (e); @@ -293,7 +296,7 @@ void cache_entry_raw_and_treeobj_tests (void) "cache_entry_create works"); ok (cache_entry_set_raw (e, NULL, 0) == 0, "cache_entry_set_raw success"); - ok ((otmp = cache_entry_get_treeobj (e)) == NULL, + ok (cache_entry_get_treeobj (e) == NULL, "cache_entry_get_treeobj returns NULL for zero length raw data"); cache_entry_destroy (e); @@ -311,7 +314,8 @@ void cache_entry_raw_and_treeobj_tests (void) ok ((otmp = cache_entry_get_treeobj (e)) != NULL, "cache_entry_get_treeobj returns non-NULL for treeobj-legal raw data"); otest = treeobj_create_val ("foo", 3); - ok (json_equal (otmp, otest) == true, + /* XXX - json_equal takes const in jansson > 2.10 */ + ok (json_equal ((json_t *)otmp, otest) == true, "treeobj returned from cache entry correct"); json_decref (o1); cache_entry_destroy (e); @@ -582,8 +586,8 @@ void cache_expiration_tests (void) tstat_t ts; int size, incomplete, dirty; json_t *o1; - json_t *otmp; json_t *otest; + const json_t *otmp; /* Put entry in cache and test lookup, expire */ @@ -638,7 +642,8 @@ void cache_expiration_tests (void) ok ((otmp = cache_entry_get_treeobj (e4)) != NULL, "cache_entry_get_treeobj found entry"); otest = treeobj_create_val ("foo", 3); - ok (json_equal (otmp, otest) == 1, + /* XXX - json_equal takes const in jansson > 2.10 */ + ok (json_equal ((json_t *)otmp, otest) == 1, "expected treeobj object found"); json_decref (otest); ok (cache_count_entries (cache) == 2,