Merge pull request #1342 from chu11/issue1337-part1

kvs: minor fixes/cleanup

src/common/libkvs/kvs_commit.c

@@ -37,6 +37,11 @@ flux_future_t *flux_kvs_fence (flux_t *h, int flags, const char *name,
 {
     const char *namespace;
 
+    if (!name || nprocs <= 0) {
+        errno = EINVAL;
+        return NULL;
+    }
+
     if (!(namespace = flux_kvs_get_namespace (h)))
         return NULL;
 

src/common/libkvs/test/kvs_commit.c

@@ -15,7 +15,11 @@ void errors (void)
 
     errno = 0;
     ok (flux_kvs_fence (NULL, 0, NULL, 0, NULL) == NULL && errno == EINVAL,
-        "flux_kvs_fence fails on bad input");
+        "flux_kvs_fence fails on bad params");
+
+    errno = 0;
+    ok (flux_kvs_fence (NULL, 0, "foo", 1, NULL) == NULL && errno == EINVAL,
+        "flux_kvs_fence fails on bad handle");
 
     errno = 0;
     ok (flux_kvs_commit (NULL, 0, NULL) == NULL && errno == EINVAL,

src/modules/kvs/commit.c

@@ -739,55 +739,53 @@ commit_process_t commit_process (commit_t *c,
              * the commit to be self-contained in the rootcpy until it
              * is unrolled later on.
              */
-            if (fence_get_json_ops (c->f)) {
-                json_t *op, *dirent;
-                const char *missing_ref = NULL;
-                json_t *ops = fence_get_json_ops (c->f);
-                int i, len = json_array_size (ops);
-                const char *key;
-                int flags;
-
-                /* Caller didn't call commit_iter_missing_refs() */
-                if (zlist_first (c->missing_refs_list))
-                    goto stall_load;
-
-                for (i = 0; i < len; i++) {
-                    missing_ref = NULL;
-                    op = json_array_get (ops, i);
-                    assert (op != NULL);
-                    if (txn_decode_op (op, &key, &flags, &dirent) < 0) {
-                        c->errnum = errno;
-                        break;
-                    }
-                    if (commit_link_dirent (c,
-                                            current_epoch,
-                                            c->rootcpy,
-                                            key,
-                                            dirent,
-                                            flags,
-                                            &missing_ref) < 0) {
-                        c->errnum = errno;
+            json_t *op, *dirent;
+            const char *missing_ref = NULL;
+            json_t *ops = fence_get_json_ops (c->f);
+            int i, len = json_array_size (ops);
+            const char *key;
+            int flags;
+
+            /* Caller didn't call commit_iter_missing_refs() */
+            if (zlist_first (c->missing_refs_list))
+                goto stall_load;
+
+            for (i = 0; i < len; i++) {
+                missing_ref = NULL;
+                op = json_array_get (ops, i);
+                assert (op != NULL);
+                if (txn_decode_op (op, &key, &flags, &dirent) < 0) {
+                    c->errnum = errno;
+                    break;
+                }
+                if (commit_link_dirent (c,
+                                        current_epoch,
+                                        c->rootcpy,
+                                        key,
+                                        dirent,
+                                        flags,
+                                        &missing_ref) < 0) {
+                    c->errnum = errno;
+                    break;
+                }
+                if (missing_ref) {
+                    if (zlist_push (c->missing_refs_list,
+                                    (void *)missing_ref) < 0) {
+                        c->errnum = ENOMEM;
                         break;
                     }
-                    if (missing_ref) {
-                        if (zlist_push (c->missing_refs_list,
-                                        (void *)missing_ref) < 0) {
-                            c->errnum = ENOMEM;
-                            break;
-                        }
-                    }
                 }
+            }
 
-                if (c->errnum != 0) {
-                    /* empty missing_refs_list to prevent mistakes later */
-                    while ((missing_ref = zlist_pop (c->missing_refs_list)));
-                    return COMMIT_PROCESS_ERROR;
-                }
+            if (c->errnum != 0) {
+                /* empty missing_refs_list to prevent mistakes later */
+                while ((missing_ref = zlist_pop (c->missing_refs_list)));
+                return COMMIT_PROCESS_ERROR;
+            }
 
-                if (zlist_first (c->missing_refs_list))
-                    goto stall_load;
+            if (zlist_first (c->missing_refs_list))
+                goto stall_load;
 
-            }
             c->state = COMMIT_STATE_STORE;
             /* fallthrough */
         }
@@ -811,7 +809,7 @@ commit_process_t commit_process (commit_t *c,
                                           false,
                                           c->newroot,
                                           &entry)) < 0)
-                     c->errnum = errno;
+                c->errnum = errno;
             else if (sret
                      && zlist_push (c->dirty_cache_entries_list, entry) < 0) {
                 commit_cleanup_dirty_cache_entry (c, entry);

src/modules/kvs/fence.c

@@ -60,10 +60,14 @@ void fence_destroy (fence_t *f)
 
 fence_t *fence_create (const char *name, int nprocs, int flags)
 {
-    fence_t *f;
+    fence_t *f = NULL;
     json_t *s = NULL;
     int saved_errno;
 
+    if (!name || nprocs <= 0) {
+        saved_errno = EINVAL;
+        goto error;
+    }
     if (!(f = calloc (1, sizeof (*f)))
         || !(f->ops = json_array ())
         || !(f->names = json_array ())
@@ -74,16 +78,14 @@ fence_t *fence_create (const char *name, int nprocs, int flags)
     f->nprocs = nprocs;
     f->flags = flags;
     f->aux_int = 0;
-    if (name) {
-        if (!(s = json_string (name))) {
-            saved_errno = ENOMEM;
-            goto error;
-        }
-        if (json_array_append_new (f->names, s) < 0) {
-            json_decref (s);
-            saved_errno = ENOMEM;
-            goto error;
-        }
+    if (!(s = json_string (name))) {
+        saved_errno = ENOMEM;
+        goto error;
+    }
+    if (json_array_append_new (f->names, s) < 0) {
+        json_decref (s);
+        saved_errno = ENOMEM;
+        goto error;
     }
 
     return f;
@@ -119,7 +121,7 @@ json_t *fence_get_json_names (fence_t *f)
     return f->names;
 }
 
-int fence_add_request_data (fence_t *f, json_t *ops)
+int fence_add_request_ops (fence_t *f, json_t *ops)
 {
     json_t *op;
     int i;

src/modules/kvs/fence.h

@@ -22,10 +22,10 @@ json_t *fence_get_json_ops (fence_t *f);
 
 json_t *fence_get_json_names (fence_t *f);
 
-/* fence_add_request_data() should be called with data on each
+/* fence_add_request_ops() should be called with ops on each
  * request, even if ops is NULL
  */
-int fence_add_request_data (fence_t *f, json_t *ops);
+int fence_add_request_ops (fence_t *f, json_t *ops);
 
 /* copy the request message into the fence, where it can be retrieved
  * later.

src/modules/kvs/kvs.c

@@ -1734,8 +1734,8 @@ static void relaycommit_request_cb (flux_t *h, flux_msg_handler_t *mh,
         goto error;
     }
 
-    if (fence_add_request_data (f, ops) < 0) {
-        flux_log_error (h, "%s: fence_add_request_data", __FUNCTION__);
+    if (fence_add_request_ops (f, ops) < 0) {
+        flux_log_error (h, "%s: fence_add_request_ops", __FUNCTION__);
         goto error;
     }
 
@@ -1809,8 +1809,8 @@ static void commit_request_cb (flux_t *h, flux_msg_handler_t *mh,
     if (fence_add_request_copy (f, msg) < 0)
         goto error;
     if (ctx->rank == 0) {
-        if (fence_add_request_data (f, ops) < 0) {
-            flux_log_error (h, "%s: fence_add_request_data", __FUNCTION__);
+        if (fence_add_request_ops (f, ops) < 0) {
+            flux_log_error (h, "%s: fence_add_request_ops", __FUNCTION__);
             goto error;
         }
 

src/modules/kvs/test/commit.c

@@ -205,8 +205,8 @@ void commit_mgr_basic_tests (void)
     ops = json_array ();
     ops_append (ops, "key1", "1", 0);
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "fence_add_request_ops add works");
 
     json_decref (ops);
 
@@ -275,8 +275,8 @@ void create_ready_commit (commit_mgr_t *cm,
     ops = json_array ();
     ops_append (ops, key, val, op_flags);
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "fence_add_request_ops add works");
 
     json_decref (ops);
 
@@ -870,8 +870,8 @@ void commit_basic_iter_not_ready_tests (void)
     ops = json_array ();
     ops_append (ops, "key1", "1", 0);
 
-    ok (fence_add_request_data (f1, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f1, ops) == 0,
+        "fence_add_request_ops add works");
 
     json_decref (ops);
 
@@ -1367,8 +1367,8 @@ void commit_process_malformed_operation (void)
     ok ((f = fence_create ("malformed", 1, 0)) != NULL,
         "fence_create works");
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "fence_add_request_ops add works");
 
     /* Submit fence_t to commit_mgr
      */

src/modules/kvs/test/fence.c

@@ -35,6 +35,9 @@ void basic_api_tests (void)
     flux_msg_t *request;
     int count = 0;
 
+    ok (fence_create (NULL, 0, 0) == NULL,
+        "fence_create fails on bad input");
+
     ok ((f = fence_create ("foo", 1, 3)) != NULL,
         "fence_create works");
 
@@ -62,18 +65,18 @@ void basic_api_tests (void)
     ops = json_array ();
     json_array_append_new (ops, json_string ("A"));
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "initial fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "initial fence_add_request_ops add works");
 
     ok ((o = fence_get_json_ops (f)) != NULL,
         "initial fence_get_json_ops call works");
 
     ok (json_equal (ops, o) == true,
         "initial fence_get_json_ops match");
 
-    ok (fence_add_request_data (f, ops) < 0
+    ok (fence_add_request_ops (f, ops) < 0
         && errno == EOVERFLOW,
-        "fence_add_request_data fails with EOVERFLOW when exceeding nprocs");
+        "fence_add_request_ops fails with EOVERFLOW when exceeding nprocs");
 
     json_decref (ops);
 
@@ -123,8 +126,8 @@ void ops_tests (void)
     ok (fence_count_reached (f) == false,
         "initial fence_count_reached() is false");
 
-    ok (fence_add_request_data (f, NULL) == 0,
-        "fence_add_request_data works with NULL ops");
+    ok (fence_add_request_ops (f, NULL) == 0,
+        "fence_add_request_ops works with NULL ops");
 
     ok (fence_count_reached (f) == false,
         "fence_count_reached() is still false");
@@ -133,8 +136,8 @@ void ops_tests (void)
     ops = json_array ();
     json_array_append_new (ops, json_string ("A"));
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "fence_add_request_ops add works");
 
     json_decref (ops);
 
@@ -145,8 +148,8 @@ void ops_tests (void)
     ops = json_array ();
     json_array_append_new (ops, json_string ("B"));
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "fence_add_request_ops add works");
 
     json_decref (ops);
 
@@ -222,8 +225,8 @@ fence_t *create_fence (const char *name, const char *opname, int flags)
     ops = json_array ();
     json_array_append_new (ops, json_string (opname));
 
-    ok (fence_add_request_data (f, ops) == 0,
-        "fence_add_request_data add works");
+    ok (fence_add_request_ops (f, ops) == 0,
+        "fence_add_request_ops add works");
 
     json_decref (ops);
 

src/modules/kvs/test/kvsroot.c

@@ -180,7 +180,7 @@ void basic_commit_mgr_tests (void)
     /* not a real operation */
     json_array_append_new (ops, json_string ("foo"));
 
-    fence_add_request_data (f, ops);
+    fence_add_request_ops (f, ops);
 
     json_decref (ops);