Merge pull request #1235 from chu11/misccleanup10

KVS: Misc cleanup, add test coverage, minor bug fix

src/modules/kvs/commit.c

@@ -684,7 +684,7 @@ commit_process_t commit_process (commit_t *c,
     return COMMIT_PROCESS_DIRTY_CACHE_ENTRIES;
 }
 
-int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data)
+int commit_iter_missing_refs (commit_t *c, commit_ref_f cb, void *data)
 {
     const char *ref;
     int saved_errno, rc = 0;
@@ -712,7 +712,7 @@ int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data)
 }
 
 int commit_iter_dirty_cache_entries (commit_t *c,
-                                     commit_cache_entry_cb cb,
+                                     commit_cache_entry_f cb,
                                      void *data)
 {
     struct cache_entry *hp;

src/modules/kvs/commit.h

@@ -22,9 +22,9 @@ typedef enum {
  * commit_t API
  */
 
-typedef int (*commit_ref_cb)(commit_t *c, const char *ref, void *data);
+typedef int (*commit_ref_f)(commit_t *c, const char *ref, void *data);
 
-typedef int (*commit_cache_entry_cb)(commit_t *c,
+typedef int (*commit_cache_entry_f)(commit_t *c,
                                      struct cache_entry *hp,
                                      void *data);
 
@@ -75,15 +75,15 @@ commit_process_t commit_process (commit_t *c,
  *
  * return -1 in callback to break iteration
  */
-int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data);
+int commit_iter_missing_refs (commit_t *c, commit_ref_f cb, void *data);
 
 /* on commit stall, iterate through all dirty cache entries that need
  * to be pushed to the content store.
  *
  * return -1 in callback to break iteration
  */
 int commit_iter_dirty_cache_entries (commit_t *c,
-                                     commit_cache_entry_cb cb,
+                                     commit_cache_entry_f cb,
                                      void *data);
 
 /* convenience function for cleaning up a dirty cache entry that was

src/modules/kvs/kvs.c

@@ -516,7 +516,7 @@ static void commit_apply (commit_t *c)
     int errnum = 0;
     commit_process_t ret;
 
-    if (commit_get_aux_errnum (c))
+    if ((errnum = commit_get_aux_errnum (c)))
         goto done;
 
     if ((ret = commit_process (c,

src/modules/kvs/test/cache.c

@@ -81,7 +81,7 @@ void cache_entry_basic_tests (void)
     ok ((otmp = cache_entry_get_json (e)) == NULL,
         "cache_entry_get_json returns NULL, no json set");
     ok (cache_entry_get_raw (e, NULL) == NULL,
-        "cache_entry_get_json returns NULL, no json set");
+        "cache_entry_get_raw returns NULL, no data set");
     cache_entry_destroy (e);
     e = NULL;
 }
@@ -332,7 +332,7 @@ void cache_entry_raw_tests (void)
     e = NULL;
 }
 
-void waiter_tests (void)
+void waiter_json_tests (void)
 {
     struct cache_entry *e;
     json_t *o;
@@ -401,6 +401,74 @@ void waiter_tests (void)
     cache_entry_destroy (e); /* destroys o */
 }
 
+void waiter_raw_tests (void)
+{
+    struct cache_entry *e;
+    char *data;
+    wait_t *w;
+    int count;
+
+    /* Test cache entry waiters.
+     * N.B. waiter is destroyed when run.
+     */
+    count = 0;
+    ok ((w = wait_create (wait_cb, &count)) != NULL,
+        "wait_create works");
+    ok ((e = cache_entry_create ()) != NULL,
+        "cache_entry_create created empty object");
+    ok (cache_entry_get_valid (e) == false,
+        "cache entry invalid, adding waiter");
+    ok (cache_entry_clear_dirty (e) < 0,
+        "cache_entry_clear_dirty returns error, b/c no object set");
+    ok (cache_entry_force_clear_dirty (e) < 0,
+        "cache_entry_force_clear_dirty returns error, b/c no object set");
+    ok (cache_entry_wait_valid (e, w) == 0,
+        "cache_entry_wait_valid success");
+    data = strdup ("abcd");
+    ok (cache_entry_set_raw (e, data, 4) == 0,
+        "cache_entry_set_raw success");
+    ok (cache_entry_get_valid (e) == true,
+        "cache entry set valid with one waiter");
+    ok (count == 1,
+        "waiter callback ran");
+
+    count = 0;
+    ok ((w = wait_create (wait_cb, &count)) != NULL,
+        "wait_create works");
+    ok (cache_entry_set_dirty (e, true) == 0,
+        "cache_entry_set_dirty success");
+    ok (cache_entry_get_dirty (e) == true,
+        "cache entry set dirty, adding waiter");
+    ok (cache_entry_wait_notdirty (e, w) == 0,
+        "cache_entry_wait_notdirty success");
+    ok (cache_entry_clear_dirty (e) == 1,
+        "cache_entry_clear_dirty returns 1, b/c of a waiter");
+    ok (cache_entry_set_dirty (e, false) == 0,
+        "cache_entry_set_dirty success");
+    ok (cache_entry_get_dirty (e) == false,
+        "cache entry set not dirty with one waiter");
+    ok (count == 1,
+        "waiter callback ran");
+
+    count = 0;
+    ok ((w = wait_create (wait_cb, &count)) != NULL,
+        "wait_create works");
+    ok (cache_entry_set_dirty (e, true) == 0,
+        "cache_entry_set_dirty success");
+    ok (cache_entry_get_dirty (e) == true,
+        "cache entry set dirty, adding waiter");
+    ok (cache_entry_wait_notdirty (e, w) == 0,
+        "cache_entry_wait_notdirty success");
+    ok (cache_entry_force_clear_dirty (e) == 0,
+        "cache_entry_clear_dirty returns 0 w/ waiter");
+    ok (cache_entry_get_dirty (e) == false,
+        "cache entry set not dirty with one waiter");
+    ok (count == 0,
+        "waiter callback not called on force clear dirty");
+
+    cache_entry_destroy (e); /* destroys o */
+}
+
 void cache_remove_entry_tests (void)
 {
     struct cache *cache;
@@ -600,7 +668,8 @@ int main (int argc, char *argv[])
     cache_entry_basic_tests ();
     cache_entry_json_tests ();
     cache_entry_raw_tests ();
-    waiter_tests ();
+    waiter_json_tests ();
+    waiter_raw_tests ();
     cache_expiration_tests ();
     cache_remove_entry_tests ();
 

src/modules/kvs/test/commit.c

@@ -827,12 +827,17 @@ void commit_process_missing_ref (void) {
 
 int ref_error_cb (commit_t *c, const char *ref, void *data)
 {
+    /* pick a weird errno */
+    errno = ENOTTY;
     return -1;
 }
 
 int cache_error_cb (commit_t *c, struct cache_entry *hp, void *data)
 {
     commit_cleanup_dirty_cache_entry (c, hp);
+
+    /* pick a weird errno */
+    errno = EXDEV;
     return -1;
 }
 
@@ -885,8 +890,10 @@ void commit_process_error_callbacks (void) {
     ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_LOAD_MISSING_REFS,
         "commit_process returns COMMIT_PROCESS_LOAD_MISSING_REFS");
 
-    ok (commit_iter_missing_refs (c, ref_error_cb, NULL) < 0,
-        "commit_iter_missing_refs errors on callback error");
+    errno = 0;
+    ok (commit_iter_missing_refs (c, ref_error_cb, NULL) < 0
+        && errno == ENOTTY,
+        "commit_iter_missing_refs errors on callback error & returns correct errno");
 
     /* insert cache entry now, want don't want missing refs on next
      * commit_process call */
@@ -895,8 +902,10 @@ void commit_process_error_callbacks (void) {
     ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_DIRTY_CACHE_ENTRIES,
         "commit_process returns COMMIT_PROCESS_DIRTY_CACHE_ENTRIES");
 
-    ok (commit_iter_dirty_cache_entries (c, cache_error_cb, NULL) < 0,
-        "commit_iter_dirty_cache_entries errors on callback error");
+    errno = 0;
+    ok (commit_iter_dirty_cache_entries (c, cache_error_cb, NULL) < 0
+        && errno == EXDEV,
+        "commit_iter_dirty_cache_entries errors on callback error & returns correct errno");
 
     commit_mgr_destroy (cm);
     cache_destroy (cache);
@@ -914,6 +923,8 @@ int cache_error_partway_cb (commit_t *c, struct cache_entry *hp, void *data)
     if (epd->total_calls > 1)
         return -1;
     epd->success_returns++;
+    /* pick a weird errno */
+    errno = EDOM;
     return 0;
 }
 
@@ -972,8 +983,10 @@ void commit_process_error_callbacks_partway (void) {
     ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_DIRTY_CACHE_ENTRIES,
         "commit_process returns COMMIT_PROCESS_DIRTY_CACHE_ENTRIES");
 
-    ok (commit_iter_dirty_cache_entries (c, cache_error_partway_cb, &epd) < 0,
-        "commit_iter_dirty_cache_entries errors on callback error");
+    errno = 0;
+    ok (commit_iter_dirty_cache_entries (c, cache_error_partway_cb, &epd) < 0
+        && errno == EDOM,
+        "commit_iter_dirty_cache_entries errors on callback error & returns correct errno");
 
     ok (epd.total_calls == 2,
         "correct number of total calls to dirty cache callback");