Skip to content

Commit

Permalink
modules/kvs: Refactor cache_entry_get_treeobj()
Browse files Browse the repository at this point in the history
Return const json_t * from function, as we do not want users
to modify any json object stored in the cache.
  • Loading branch information
chu11 committed Nov 3, 2017
1 parent e262dab commit bc0a6e2
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/modules/kvs/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/modules/kvs/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions src/modules/kvs/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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))
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/modules/kvs/kvs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 7 additions & 10 deletions src/modules/kvs/lookup.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 */
Expand All @@ -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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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. */
Expand All @@ -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
*/
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 14 additions & 9 deletions src/modules/kvs/test/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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");
Expand Down Expand Up @@ -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);

Expand All @@ -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;

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit bc0a6e2

Please sign in to comment.