Merge pull request #1273 from chu11/issue1264-part3

kvs: additional cleanup and refactoring

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();
 }

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
  */

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 */
 
 /*

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;

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);

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;
 
@@ -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;
         }
     }
@@ -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;

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));
@@ -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;
     }
@@ -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) {

src/modules/kvs/kvs_util.c

@@ -27,68 +27,8 @@
 #endif
 #include <stdio.h>
 #include <stdlib.h>
-#include <errno.h>
-#include <unistd.h>
 #include <stdbool.h>
-#include <ctype.h>
-#include <flux/core.h>
-#include <jansson.h>
-
-#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 (and reference when
-     * used by kvs_util_json_hash()).
-     */
-    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;
-}
-
-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;
-}
+#include <string.h>
 
 char *kvs_util_normalize_key (const char *key, bool *want_directory)
 {

src/modules/kvs/kvs_util.h

@@ -1,27 +1,5 @@
 #ifndef _FLUX_KVS_UTIL_H
 #define _FLUX_KVS_UTIL_H
-#include <jansson.h>
-
-#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, especially
- * when dealing with data that may be hashed via kvs_util_json_hash().
- *
- * 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);
-
-/* 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.

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;

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");