From 5cab0d829b92f9f2625aad60785eb9a86995a7c4 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Fri, 20 Oct 2017 14:25:35 -0700
Subject: [PATCH 01/17] libkvs/txn: fix memleak in op construction

Problem: if validate_op() fails when adding operation
to transaction, the operation is leaked.

Ensure json_decref() is called on this error path.
---
 src/common/libkvs/kvs_txn.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c
index e3ce3af5db49..330e57185218 100644
--- a/src/common/libkvs/kvs_txn.c
+++ b/src/common/libkvs/kvs_txn.c
@@ -122,19 +122,22 @@ static int validate_op (json_t *op)
 static int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags,
                                      const char *key, json_t *dirent)
 {
-    json_t *op;
+    json_t *op = NULL;
+    int saved_errno;
 
     if (txn_encode_op (key, flags, dirent, &op) < 0)
         goto error;
     if (validate_op (op) < 0)
         goto error;
     if (json_array_append_new (txn->ops, op) < 0) {
-        json_decref (op);
         errno = ENOMEM;
         goto error;
     }
     return 0;
 error:
+    saved_errno = errno;
+    json_decref (op);
+    errno = saved_errno;
     return -1;
 }
 

From 08ee320317c0e7b6314253f5747722576c21431b Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Fri, 20 Oct 2017 16:11:06 -0700
Subject: [PATCH 02/17] libkvs/lookup: add flux_kvs_lookup_get_symlink()

Create a function specifically for retrieving a link
target looked up with FLUX_KVS_READLINK, instead of
overloading flux_kvs_lookup_get().

Update users in lua bindings, flux-kvs command,
and tests.
---
 src/bindings/lua/flux-lua.c    |  2 +-
 src/cmd/flux-kvs.c             |  4 +--
 src/common/libkvs/kvs_lookup.c | 52 +++++++++++++++++++++-------------
 src/common/libkvs/kvs_lookup.h |  1 +
 t/kvs/basic.c                  |  4 +--
 5 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/src/bindings/lua/flux-lua.c b/src/bindings/lua/flux-lua.c
index 119507860a9b..5ff2d35dd4e7 100644
--- a/src/bindings/lua/flux-lua.c
+++ b/src/bindings/lua/flux-lua.c
@@ -330,7 +330,7 @@ static int l_flux_kvs_type (lua_State *L)
         return lua_pusherror (L, "key expected in arg #2");
 
     if ((future = flux_kvs_lookup (f, FLUX_KVS_READLINK, key))
-            && flux_kvs_lookup_get (future, &target) == 0) {
+            && flux_kvs_lookup_get_symlink (future, &target) == 0) {
         lua_pushstring (L, "symlink");
         lua_pushstring (L, target);
         flux_future_destroy (future);
diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c
index 0f05c93b3983..d77ae7ddc2b6 100644
--- a/src/cmd/flux-kvs.c
+++ b/src/cmd/flux-kvs.c
@@ -506,7 +506,7 @@ int cmd_readlink (optparse_t *p, int argc, char **argv)
 
     for (i = optindex; i < argc; i++) {
         if (!(f = flux_kvs_lookup (h, FLUX_KVS_READLINK, argv[i]))
-                || flux_kvs_lookup_get (f, &target) < 0)
+                || flux_kvs_lookup_get_symlink (f, &target) < 0)
             log_err_exit ("%s", argv[i]);
         else
             printf ("%s\n", target);
@@ -781,7 +781,7 @@ static void dump_kvs_dir (const flux_kvsdir_t *dir, bool Ropt, bool dopt)
         if (flux_kvsdir_issymlink (dir, name)) {
             const char *link;
             if (!(f = flux_kvs_lookupat (h, FLUX_KVS_READLINK, key, rootref))
-                    || flux_kvs_lookup_get (f, &link) < 0)
+                    || flux_kvs_lookup_get_symlink (f, &link) < 0)
                 log_err_exit ("%s", key);
             printf ("%s -> %s\n", key, link);
             flux_future_destroy (f);
diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c
index 87736c5609cc..63f8da49d7f2 100644
--- a/src/common/libkvs/kvs_lookup.c
+++ b/src/common/libkvs/kvs_lookup.c
@@ -192,28 +192,10 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **json_str)
         if (json_str)
             *json_str = ctx->treeobj_str;
     }
-    /* If SYMLINK flag, extract link target from symlink object
-     * and return it directly.
-     */
-    else if ((ctx->flags & FLUX_KVS_READLINK)) {
-        if (!treeobj_is_symlink (ctx->treeobj)) {
-            errno = EINVAL;
-            return -1;
-        }
-        if (json_str) {
-            json_t *str = treeobj_get_data (ctx->treeobj);
-            const char *s = json_string_value (str);
-            if (!s) {
-                errno = EINVAL;
-                return -1;
-            }
-            *json_str = s;
-        }
-    }
     /* No flags, val is a 'val' object.
      * Decide the data and return it as a string if it is properly terminated.
      */
-    else {
+    else if (ctx->flags == 0) {
         if (!ctx->val_valid) {
             if (treeobj_decode_val (ctx->treeobj, &ctx->val_data,
                                                   &ctx->val_len) < 0)
@@ -228,6 +210,10 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **json_str)
                 *json_str = s;
         }
     }
+    else {
+        errno = EINVAL;
+        return -1;
+    }
     return 0;
 }
 
@@ -306,6 +292,34 @@ int flux_kvs_lookup_get_dir (flux_future_t *f, const flux_kvsdir_t **dirp)
     return 0;
 }
 
+int flux_kvs_lookup_get_symlink (flux_future_t *f, const char **target)
+{
+    struct lookup_ctx *ctx;
+    json_t *str;
+    const char *s;
+
+    if (!(ctx = flux_future_aux_get (f, auxkey))) {
+        errno = EINVAL;
+        return -1;
+    }
+    if (!(ctx->treeobj)) {
+        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+            return -1;
+    }
+    if (!treeobj_is_symlink (ctx->treeobj)) {
+        errno = EINVAL;
+        return -1;
+    }
+    if (!(str = treeobj_get_data (ctx->treeobj))
+                                || !(s = json_string_value (str))) {
+        errno = EINVAL;
+        return -1;
+    }
+    if (target)
+        *target = s;
+    return 0;
+}
+
 /*
  * vi:tabstop=4 shiftwidth=4 expandtab
  */
diff --git a/src/common/libkvs/kvs_lookup.h b/src/common/libkvs/kvs_lookup.h
index ed8827c6a831..4cb9662c9a87 100644
--- a/src/common/libkvs/kvs_lookup.h
+++ b/src/common/libkvs/kvs_lookup.h
@@ -13,6 +13,7 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **json_str);
 int flux_kvs_lookup_get_unpack (flux_future_t *f, const char *fmt, ...);
 int flux_kvs_lookup_get_raw (flux_future_t *f, const void **data, int *len);
 int flux_kvs_lookup_get_dir (flux_future_t *f, const flux_kvsdir_t **dir);
+int flux_kvs_lookup_get_symlink (flux_future_t *f, const char **target);
 
 #ifdef __cplusplus
 }
diff --git a/t/kvs/basic.c b/t/kvs/basic.c
index ea92f9e7426a..42542bef94da 100644
--- a/t/kvs/basic.c
+++ b/t/kvs/basic.c
@@ -343,7 +343,7 @@ static void dump_kvs_dir (const flux_kvsdir_t *dir, bool ropt)
         if (flux_kvsdir_issymlink (dir, name)) {
             const char *link;
             if (!(f = flux_kvs_lookupat (h, FLUX_KVS_READLINK, key, rootref))
-                    || flux_kvs_lookup_get (f, &link) < 0)
+                    || flux_kvs_lookup_get_symlink (f, &link) < 0)
                 log_err_exit ("%s", key);
             printf ("%s -> %s\n", key, link);
             flux_future_destroy (f);
@@ -472,7 +472,7 @@ void cmd_readlinkat (flux_t *h, int argc, char **argv)
     if (argc != 2)
         log_msg_exit ("readlink: specify treeobj and key");
     if (!(f = flux_kvs_lookupat (h, FLUX_KVS_READLINK, argv[1], argv[0]))
-            || flux_kvs_lookup_get (f, &target) < 0)
+            || flux_kvs_lookup_get_symlink (f, &target) < 0)
         log_err_exit ("%s", argv[1]);
     else
         printf ("%s\n", target);

From 825fbf6e4bf5702e1c30ee696688d33b565b4b1c Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 10:24:52 -0700
Subject: [PATCH 03/17] libkvs/dir: [cleanup] add private interface

Add
  kvsdir_get_obj()
  kvsdir_create_fromobj()

Prepare for future cleanup that will avoid extra encode/decode
steps within the KVS API implementation.

These functions are defined in a private header and are not
exported as part of the flux public API.
---
 src/common/libkvs/Makefile.am       |  1 +
 src/common/libkvs/kvs_dir.c         | 80 +++++++++++++++++------------
 src/common/libkvs/kvs_dir_private.h | 14 +++++
 3 files changed, 63 insertions(+), 32 deletions(-)
 create mode 100644 src/common/libkvs/kvs_dir_private.h

diff --git a/src/common/libkvs/Makefile.am b/src/common/libkvs/Makefile.am
index 97ad2de2ee51..4c00a17ec298 100644
--- a/src/common/libkvs/Makefile.am
+++ b/src/common/libkvs/Makefile.am
@@ -15,6 +15,7 @@ libkvs_la_SOURCES = \
 	kvs.c \
 	kvs_lookup.c \
 	kvs_dir.c \
+	kvs_dir_private.h \
 	kvs_classic.c \
 	kvs_watch.c \
 	kvs_commit.c \
diff --git a/src/common/libkvs/kvs_dir.c b/src/common/libkvs/kvs_dir.c
index 5e82ed204beb..47cb9e049ddc 100644
--- a/src/common/libkvs/kvs_dir.c
+++ b/src/common/libkvs/kvs_dir.c
@@ -32,6 +32,7 @@
 #include <czmq.h>
 #include <flux/core.h>
 
+#include "kvs_dir_private.h"
 #include "treeobj.h"
 
 struct flux_kvsdir {
@@ -73,46 +74,24 @@ flux_kvsdir_t *flux_kvsdir_copy (const flux_kvsdir_t *dir)
 }
 
 /* If rootref is non-NULL, the kvsdir records the root reference
- * so that subsequent kvsdir_get_* accesses can be relative to that
+ * so that subsequent flux_kvsdir_get_* accesses can be relative to that
  * snapshot.  Otherwise, they are relative to the current root.
  */
-flux_kvsdir_t *flux_kvsdir_create (flux_t *handle, const char *rootref,
+flux_kvsdir_t *flux_kvsdir_create (flux_t *h, const char *rootref,
                                    const char *key, const char *json_str)
 {
-    flux_kvsdir_t *dir;
+    flux_kvsdir_t *dir = NULL;
+    json_t *dirobj = NULL;
 
-    if (!key || !json_str) {
+    if (!key || !json_str || !(dirobj = treeobj_decode (json_str))) {
         errno = EINVAL;
-        return NULL;
+        goto done;
     }
-    if (!(dir = calloc (1, sizeof (*dir))))
-        goto error;
-
-    dir->handle = handle;
-    if (rootref) {
-        if (!(dir->rootref = strdup (rootref)))
-            goto error;
-    }
-    if (!(dir->key = strdup (key)))
-        goto error;
-    if (!(dir->json_str = strdup (json_str)))
-        goto error;
-    if (!(dir->dirobj = json_loads (json_str, 0, NULL))) {
-        errno = EINVAL;
-        goto error;
-    }
-    if (treeobj_validate (dir->dirobj) < 0)
-        goto error;
-    if (!treeobj_is_dir (dir->dirobj)) {
-        errno = EINVAL;
-        goto error;
-    }
-    dir->usecount = 1;
-
+    if (!(dir = kvsdir_create_fromobj (h, rootref, key, dirobj)))
+        goto done;
+done:
+    json_decref (dirobj);
     return dir;
-error:
-    flux_kvsdir_destroy (dir);
-    return NULL;
 }
 
 const char *flux_kvsdir_tostring (const flux_kvsdir_t *dir)
@@ -257,6 +236,43 @@ char *flux_kvsdir_key_at (const flux_kvsdir_t *dir, const char *name)
     return NULL;
 }
 
+/* kvs_txn_private.h */
+
+flux_kvsdir_t *kvsdir_create_fromobj (flux_t *handle, const char *rootref,
+                                      const char *key, json_t *treeobj)
+{
+    flux_kvsdir_t *dir = NULL;
+
+    if (!key || !treeobj || treeobj_validate (treeobj) < 0
+                         || !treeobj_is_dir (treeobj)) {
+        errno = EINVAL;
+        goto error;
+    }
+    if (!(dir = calloc (1, sizeof (*dir))))
+        goto error;
+
+    dir->handle = handle;
+    if (rootref) {
+        if (!(dir->rootref = strdup (rootref)))
+            goto error;
+    }
+    if (!(dir->key = strdup (key)))
+        goto error;
+    if (!(dir->json_str = treeobj_encode (treeobj)))
+        goto error;
+    dir->dirobj = json_incref (treeobj);
+    dir->usecount = 1;
+
+    return dir;
+error:
+    flux_kvsdir_destroy (dir);
+    return NULL;
+}
+
+json_t *kvsdir_get_obj (flux_kvsdir_t *dir)
+{
+    return dir->dirobj;
+}
 
 /*
  * vi:tabstop=4 shiftwidth=4 expandtab
diff --git a/src/common/libkvs/kvs_dir_private.h b/src/common/libkvs/kvs_dir_private.h
new file mode 100644
index 000000000000..0a6096243d9e
--- /dev/null
+++ b/src/common/libkvs/kvs_dir_private.h
@@ -0,0 +1,14 @@
+#ifndef _KVS_DIR_PRIVATE_H
+#define _KVS_DIR_PRIVATE_H
+
+flux_kvsdir_t *kvsdir_create_fromobj (flux_t *handle, const char *rootref,
+                                      const char *key, json_t *treeobj);
+
+json_t *kvsdir_get_obj (flux_kvsdir_t *dir);
+
+
+#endif /* !_KVS_DIR_PRIVATE_H */
+
+/*
+ * vi:tabstop=4 shiftwidth=4 expandtab
+ */

From 1478ea06b28ba7ae5a2ebc772c174842a97fdb4e Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 10:14:45 -0700
Subject: [PATCH 04/17] libkvs/lookup: [cleanup] reduce decoding/encoding

Problem: flux_kvs_lookup_get_dir() internally calls
flux_kvs_lookup_get() which returns the directory object
encoded as a string, then passes it to flux_kvsdir_create(),
which decodes it back into an object.

Make flux_kvs_lookup_get_dir() independent of flux_kvs_lookup_get()
and use flux_kvsdir_create_fromobj() to avoid the extra string
encoding.
---
 src/common/libkvs/kvs_lookup.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c
index 63f8da49d7f2..6972b024c00f 100644
--- a/src/common/libkvs/kvs_lookup.c
+++ b/src/common/libkvs/kvs_lookup.c
@@ -32,6 +32,7 @@
 #include <czmq.h>
 #include <flux/core.h>
 
+#include "kvs_dir_private.h"
 #include "kvs_lookup.h"
 #include "treeobj.h"
 
@@ -279,12 +280,13 @@ int flux_kvs_lookup_get_dir (flux_future_t *f, const flux_kvsdir_t **dirp)
         errno = EINVAL;
         return -1;
     }
-    if (!ctx->dir) {
-        const char *json_str;
-        if (flux_kvs_lookup_get (f, &json_str) < 0)
+    if (!(ctx->treeobj)) {
+        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
             return -1;
-        ctx->dir = flux_kvsdir_create (ctx->h, ctx->atref, ctx->key, json_str);
-        if (!ctx->dir)
+    }
+    if (!ctx->dir) {
+        if (!(ctx->dir = kvsdir_create_fromobj (ctx->h, ctx->atref,
+                                                ctx->key, ctx->treeobj)))
             return -1;
     }
     if (dirp)

From eee3b714ca615e0971030cebc2fe1e7850383ebe Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 12:38:49 -0700
Subject: [PATCH 05/17] libkvs/dir: [cleanup] reduce decoding/encoding

Problem: flux_kvsdir_copy() calls flux_kvsdir_create()
which only accepts an encoded directory object.

Change flux_kvsdir_copy to call kvsdir_create_fromobj(),
which accepts the source directory object directly, incrementing
its reference count instead of recreating it.
---
 src/common/libkvs/kvs_dir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/common/libkvs/kvs_dir.c b/src/common/libkvs/kvs_dir.c
index 47cb9e049ddc..72bfade32309 100644
--- a/src/common/libkvs/kvs_dir.c
+++ b/src/common/libkvs/kvs_dir.c
@@ -69,8 +69,8 @@ void flux_kvsdir_destroy (flux_kvsdir_t *dir)
 
 flux_kvsdir_t *flux_kvsdir_copy (const flux_kvsdir_t *dir)
 {
-    return flux_kvsdir_create (dir->handle, dir->rootref,
-                               dir->key, dir->json_str);
+    return kvsdir_create_fromobj (dir->handle, dir->rootref,
+                                  dir->key, dir->dirobj);
 }
 
 /* If rootref is non-NULL, the kvsdir records the root reference

From 8f0982ed43c4821b6efcf614b37ae0b4d3599522 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 13:00:51 -0700
Subject: [PATCH 06/17] libkvs/dir: [cleanup] drop flux_kvsdir_tostring

Problem: flux_kvsdir_tostring() has only one internal user.

Change flux_kvs_watch_once_dir() to call kvsdir_get_obj()
and treeobj_encode() instead, then drop flux_kvsdir_tostring()
and its usage in the kvsdir unit test.
---
 src/common/libkvs/kvs_dir.c      | 9 ---------
 src/common/libkvs/kvs_dir.h      | 1 -
 src/common/libkvs/kvs_watch.c    | 8 +++++---
 src/common/libkvs/test/kvs_dir.c | 5 +++--
 4 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/src/common/libkvs/kvs_dir.c b/src/common/libkvs/kvs_dir.c
index 72bfade32309..4bbc07febaed 100644
--- a/src/common/libkvs/kvs_dir.c
+++ b/src/common/libkvs/kvs_dir.c
@@ -39,7 +39,6 @@ struct flux_kvsdir {
     flux_t *handle;
     char *rootref; /* optional snapshot reference */
     char *key;
-    char *json_str;
     json_t *dirobj;
     int usecount;
 };
@@ -60,7 +59,6 @@ void flux_kvsdir_destroy (flux_kvsdir_t *dir)
         int saved_errno = errno;
         free (dir->rootref);
         free (dir->key);
-        free (dir->json_str);
         json_decref (dir->dirobj);
         free (dir);
         errno = saved_errno;
@@ -94,11 +92,6 @@ flux_kvsdir_t *flux_kvsdir_create (flux_t *h, const char *rootref,
     return dir;
 }
 
-const char *flux_kvsdir_tostring (const flux_kvsdir_t *dir)
-{
-    return dir->json_str;
-}
-
 int flux_kvsdir_get_size (const flux_kvsdir_t *dir)
 {
     return treeobj_get_count (dir->dirobj);
@@ -258,8 +251,6 @@ flux_kvsdir_t *kvsdir_create_fromobj (flux_t *handle, const char *rootref,
     }
     if (!(dir->key = strdup (key)))
         goto error;
-    if (!(dir->json_str = treeobj_encode (treeobj)))
-        goto error;
     dir->dirobj = json_incref (treeobj);
     dir->usecount = 1;
 
diff --git a/src/common/libkvs/kvs_dir.h b/src/common/libkvs/kvs_dir.h
index af5401686861..ad6cc4e9aa2f 100644
--- a/src/common/libkvs/kvs_dir.h
+++ b/src/common/libkvs/kvs_dir.h
@@ -66,7 +66,6 @@ bool flux_kvsdir_issymlink (const flux_kvsdir_t *dir, const char *name);
  * passed to flux_kvsdir_create ()
  */
 const char *flux_kvsdir_key (const flux_kvsdir_t *dir);
-const char *flux_kvsdir_tostring (const flux_kvsdir_t *dir);
 void *flux_kvsdir_handle (const flux_kvsdir_t *dir);
 const char *flux_kvsdir_rootref (const flux_kvsdir_t *dir);
 
diff --git a/src/common/libkvs/kvs_watch.c b/src/common/libkvs/kvs_watch.c
index 0c155f4ff9aa..e3fb6e6db8aa 100644
--- a/src/common/libkvs/kvs_watch.c
+++ b/src/common/libkvs/kvs_watch.c
@@ -30,6 +30,7 @@
 #include <czmq.h>
 
 #include "treeobj.h"
+#include "kvs_dir_private.h"
 
 typedef enum {
     WATCH_JSONSTR, WATCH_DIR,
@@ -370,7 +371,7 @@ int flux_kvs_watch_once (flux_t *h, const char *key, char **valp)
 
 static int watch_once_dir (flux_t *h, const char *key, flux_kvsdir_t **dirp)
 {
-    const char *val_in = NULL;
+    char *val_in = NULL;
     char *val_out = NULL;
     flux_kvsdir_t *dir_out = NULL;
     flux_future_t *f = NULL;
@@ -381,8 +382,8 @@ static int watch_once_dir (flux_t *h, const char *key, flux_kvsdir_t **dirp)
         goto done;
     }
     if (*dirp) {
-        if (!(val_in = flux_kvsdir_tostring (*dirp)))
-            goto done;
+        json_t *dir_in = kvsdir_get_obj (*dirp);
+        val_in = treeobj_encode (dir_in);
     }
     if (!(f = kvs_watch_rpc (h, key, val_in,
                              KVS_WATCH_ONCE | FLUX_KVS_READDIR)))
@@ -399,6 +400,7 @@ static int watch_once_dir (flux_t *h, const char *key, flux_kvsdir_t **dirp)
     rc = 0;
 done:
     free (val_out);
+    free (val_in);
     flux_future_destroy (f);
     return rc;
 }
diff --git a/src/common/libkvs/test/kvs_dir.c b/src/common/libkvs/test/kvs_dir.c
index 938093ace406..bd06573a6430 100644
--- a/src/common/libkvs/test/kvs_dir.c
+++ b/src/common/libkvs/test/kvs_dir.c
@@ -8,6 +8,7 @@
 
 #include "kvs.h"
 #include "kvs_txn_private.h"
+#include "kvs_dir_private.h"
 #include "treeobj.h"
 
 #include "src/common/libflux/flux.h"
@@ -68,7 +69,7 @@ void test_empty (void)
 
     ok (dir != NULL,
         "flux_kvsdir_create with empty directory works");
-    diag ("%s", flux_kvsdir_tostring (dir));
+    jdiag (kvsdir_get_obj (dir));
 
     ok (!flux_kvsdir_exists (dir, "noexist"),
         "flux_kvsdir_exists on nonexistent key returns false");
@@ -146,7 +147,7 @@ void test_full (void)
     free (s);
     ok (dir != NULL,
         "flux_kvsdir_create works");
-    diag ("%s", flux_kvsdir_tostring (dir));
+    jdiag (kvsdir_get_obj (dir));
 
     ok (!flux_kvsdir_exists (dir, "noexist"),
         "flux_kvsdir_exists on nonexistent key returns false");

From 3e50239129b46962d5509d3ac7501fe44542eaeb Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 14:23:23 -0700
Subject: [PATCH 07/17] libkvs/lookup: [cleanup] make get_unpack independent

Problem: flux_kvs_lookup_get_unpack() calls flux_kvs_lookup_get()
internally, make the code hard to follow.

Add the small amount of code to allow flux_kvs_lookup_unpack()
to stand alone.
---
 src/common/libkvs/kvs_lookup.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c
index 6972b024c00f..92ec6383bb51 100644
--- a/src/common/libkvs/kvs_lookup.c
+++ b/src/common/libkvs/kvs_lookup.c
@@ -167,6 +167,14 @@ flux_future_t *flux_kvs_lookupat (flux_t *h, int flags, const char *key,
     return f;
 }
 
+static bool value_is_string (const char *s, int len)
+{
+    if (len < 1 || s[len - 1] != '\0')
+        return false;
+    else
+        return true;
+}
+
 int flux_kvs_lookup_get (flux_future_t *f, const char **json_str)
 {
     struct lookup_ctx *ctx;
@@ -202,13 +210,12 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **json_str)
                                                   &ctx->val_len) < 0)
                 return -1;
             ctx->val_valid = true;
-            char *s = ctx->val_data;
-            if (ctx->val_len < 1 || s[ctx->val_len - 1] != '\0') {
+            if (!value_is_string (ctx->val_data, ctx->val_len)) {
                 errno = EINVAL;
                 return -1;
             }
             if (json_str)
-                *json_str = s;
+                *json_str = ctx->val_data;
         }
     }
     else {
@@ -228,11 +235,23 @@ int flux_kvs_lookup_get_unpack (flux_future_t *f, const char *fmt, ...)
         errno = EINVAL;
         return -1;
     }
+    if (!(ctx->treeobj)) {
+        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+            return -1;
+    }
+    if (!ctx->val_valid) {
+        if (treeobj_decode_val (ctx->treeobj, &ctx->val_data,
+                                              &ctx->val_len) < 0)
+            return -1;
+        ctx->val_valid = true;
+    }
     if (!ctx->val_obj) {
-        const char *json_str;
-        if (flux_kvs_lookup_get (f, &json_str) < 0)
+        if (!value_is_string (ctx->val_data, ctx->val_len)) {
+            errno = EINVAL;
             return -1;
-        if (!(ctx->val_obj = json_loads (json_str, JSON_DECODE_ANY, NULL))) {
+        }
+        if (!(ctx->val_obj = json_loads (ctx->val_data,
+                                         JSON_DECODE_ANY, NULL))) {
             errno = EINVAL;
             return -1;
         }

From 55baa5887b71b996eb074808d9f0b3a2be8d343e Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 14:45:01 -0700
Subject: [PATCH 08/17] libkvs/lookup: add flux_kvs_lookup_get_treeobj

Create a function specifically for retrieving a
tree object looked up with FLUX_KVS_TREEOBJ, instead of
overloading flux_kvs_lookup_get().

Update users and test cases.
---
 src/cmd/flux-kvs.c             |  6 ++--
 src/common/libjsc/jstatctl.c   |  2 +-
 src/common/libkvs/kvs_lookup.c | 62 ++++++++++++++++------------------
 src/common/libkvs/kvs_lookup.h |  3 +-
 t/kvs/basic.c                  |  2 +-
 5 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c
index d77ae7ddc2b6..173167bf2e96 100644
--- a/src/cmd/flux-kvs.c
+++ b/src/cmd/flux-kvs.c
@@ -1056,7 +1056,7 @@ static int categorize_key (optparse_t *p, const char *key,
     }
     if (!(f = flux_kvs_lookup (h, FLUX_KVS_TREEOBJ, nkey)))
         log_err_exit ("flux_kvs_lookup");
-    if (flux_kvs_lookup_get (f, &json_str) < 0) {
+    if (flux_kvs_lookup_get_treeobj (f, &json_str) < 0) {
         fprintf (stderr, "%s: %s\n", nkey, flux_strerror (errno));
         goto error;
     }
@@ -1166,7 +1166,7 @@ int cmd_copy (optparse_t *p, int argc, char **argv)
     dstkey = argv[optindex + 1];
 
     if (!(f = flux_kvs_lookup (h, FLUX_KVS_TREEOBJ, srckey))
-            || flux_kvs_lookup_get (f, &json_str) < 0)
+            || flux_kvs_lookup_get_treeobj (f, &json_str) < 0)
         log_err_exit ("flux_kvs_lookup %s", srckey);
     if (!(txn = flux_kvs_txn_create ()))
         log_err_exit ("flux_kvs_txn_create");
@@ -1198,7 +1198,7 @@ int cmd_move (optparse_t *p, int argc, char **argv)
     dstkey = argv[optindex + 1];
 
     if (!(f = flux_kvs_lookup (h, FLUX_KVS_TREEOBJ, srckey))
-            || flux_kvs_lookup_get (f, &json_str) < 0)
+            || flux_kvs_lookup_get_treeobj (f, &json_str) < 0)
         log_err_exit ("flux_kvs_lookup %s", srckey);
     if (!(txn = flux_kvs_txn_create ()))
         log_err_exit ("flux_kvs_txn_create");
diff --git a/src/common/libjsc/jstatctl.c b/src/common/libjsc/jstatctl.c
index f475cc87c282..2bcaaf5cc761 100644
--- a/src/common/libjsc/jstatctl.c
+++ b/src/common/libjsc/jstatctl.c
@@ -309,7 +309,7 @@ static int jobid_exist (flux_t *h, int64_t j)
     if (path == NULL)
         goto done;
     if (!(f = flux_kvs_lookup (h, FLUX_KVS_READDIR, path))
-                                || flux_kvs_lookup_get (f, NULL) < 0) {
+                                || flux_kvs_lookup_get_dir (f, NULL) < 0) {
         flux_log (h, LOG_DEBUG, "flux_kvs_lookup(%s): %s",
                      path, flux_strerror (errno));
         goto done;
diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c
index 92ec6383bb51..28d80dc3c507 100644
--- a/src/common/libkvs/kvs_lookup.c
+++ b/src/common/libkvs/kvs_lookup.c
@@ -37,7 +37,6 @@
 #include "treeobj.h"
 
 struct lookup_ctx {
-    int flags;
     flux_t *h;
     char *key;
     char *atref;
@@ -73,7 +72,6 @@ static struct lookup_ctx *alloc_ctx (flux_t *h, int flags, const char *key)
     if (!(ctx = calloc (1, sizeof (*ctx))))
         goto nomem;
     ctx->h = h;
-    ctx->flags = flags;
     if (!(ctx->key = strdup (key)))
         goto nomem;
     return ctx;
@@ -175,7 +173,7 @@ static bool value_is_string (const char *s, int len)
         return true;
 }
 
-int flux_kvs_lookup_get (flux_future_t *f, const char **json_str)
+int flux_kvs_lookup_get (flux_future_t *f, const char **value)
 {
     struct lookup_ctx *ctx;
 
@@ -187,41 +185,39 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **json_str)
         if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
             return -1;
     }
-    /* If TREEOBJ or READDIR flags, val is a tree object.
-     * Re-encode as a string and return.
-     */
-    if ((ctx->flags & FLUX_KVS_TREEOBJ) || (ctx->flags & FLUX_KVS_READDIR)) {
-        if (!ctx->treeobj_str) {
-            size_t flags = JSON_ENCODE_ANY | JSON_COMPACT | JSON_SORT_KEYS;
-            if (!(ctx->treeobj_str = json_dumps (ctx->treeobj, flags))) {
-                errno = EINVAL;
-                return -1;
-            }
-        }
-        if (json_str)
-            *json_str = ctx->treeobj_str;
+    if (!ctx->val_valid) {
+        if (treeobj_decode_val (ctx->treeobj, &ctx->val_data,
+                                              &ctx->val_len) < 0)
+            return -1;
+        ctx->val_valid = true;
     }
-    /* No flags, val is a 'val' object.
-     * Decide the data and return it as a string if it is properly terminated.
-     */
-    else if (ctx->flags == 0) {
-        if (!ctx->val_valid) {
-            if (treeobj_decode_val (ctx->treeobj, &ctx->val_data,
-                                                  &ctx->val_len) < 0)
-                return -1;
-            ctx->val_valid = true;
-            if (!value_is_string (ctx->val_data, ctx->val_len)) {
-                errno = EINVAL;
-                return -1;
-            }
-            if (json_str)
-                *json_str = ctx->val_data;
-        }
+    if (!value_is_string (ctx->val_data, ctx->val_len)) {
+        errno = EINVAL;
+        return -1;
     }
-    else {
+    if (value)
+        *value = ctx->val_data;
+    return 0;
+}
+
+int flux_kvs_lookup_get_treeobj (flux_future_t *f, const char **treeobj)
+{
+    struct lookup_ctx *ctx;
+
+    if (!(ctx = flux_future_aux_get (f, auxkey))) {
         errno = EINVAL;
         return -1;
     }
+    if (!(ctx->treeobj)) {
+        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+            return -1;
+    }
+    if (!ctx->treeobj_str) {
+        if (!(ctx->treeobj_str = treeobj_encode (ctx->treeobj)))
+            return -1;
+    }
+    if (treeobj)
+        *treeobj= ctx->treeobj_str;
     return 0;
 }
 
diff --git a/src/common/libkvs/kvs_lookup.h b/src/common/libkvs/kvs_lookup.h
index 4cb9662c9a87..30fd3ef52d44 100644
--- a/src/common/libkvs/kvs_lookup.h
+++ b/src/common/libkvs/kvs_lookup.h
@@ -9,9 +9,10 @@ flux_future_t *flux_kvs_lookup (flux_t *h, int flags, const char *key);
 flux_future_t *flux_kvs_lookupat (flux_t *h, int flags, const char *key,
                                   const char *treeobj);
 
-int flux_kvs_lookup_get (flux_future_t *f, const char **json_str);
+int flux_kvs_lookup_get (flux_future_t *f, const char **value);
 int flux_kvs_lookup_get_unpack (flux_future_t *f, const char *fmt, ...);
 int flux_kvs_lookup_get_raw (flux_future_t *f, const void **data, int *len);
+int flux_kvs_lookup_get_treeobj (flux_future_t *f, const char **treeobj);
 int flux_kvs_lookup_get_dir (flux_future_t *f, const flux_kvsdir_t **dir);
 int flux_kvs_lookup_get_symlink (flux_future_t *f, const char **target);
 
diff --git a/t/kvs/basic.c b/t/kvs/basic.c
index 42542bef94da..04dd0b910faa 100644
--- a/t/kvs/basic.c
+++ b/t/kvs/basic.c
@@ -420,7 +420,7 @@ void cmd_get_treeobj (flux_t *h, int argc, char **argv)
     if (argc != 1)
         log_msg_exit ("get-treeobj: specify key");
     if (!(f = flux_kvs_lookup (h, FLUX_KVS_TREEOBJ, argv[0]))
-            || flux_kvs_lookup_get (f, &treeobj) < 0)
+            || flux_kvs_lookup_get_treeobj (f, &treeobj) < 0)
         log_err_exit ("kvs_get_treeobj %s", argv[0]);
     printf ("%s\n", treeobj);
     flux_future_destroy (f);

From e82b9a778eceb3ffd526dc571d00ab678f224fd9 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Wed, 25 Oct 2017 07:14:27 -0700
Subject: [PATCH 09/17] libkvs/lookup: lookup_get can return NULL value

A flux_kvs_lookup_get() on a NULL (zero length) value
should set value to NULL and return success.
---
 src/common/libkvs/kvs_lookup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c
index 28d80dc3c507..7325521bad24 100644
--- a/src/common/libkvs/kvs_lookup.c
+++ b/src/common/libkvs/kvs_lookup.c
@@ -191,7 +191,7 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **value)
             return -1;
         ctx->val_valid = true;
     }
-    if (!value_is_string (ctx->val_data, ctx->val_len)) {
+    if (ctx->val_data && !value_is_string (ctx->val_data, ctx->val_len)) {
         errno = EINVAL;
         return -1;
     }

From 60709f83007c20ac2d974b5f7d20d3c4d68da369 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 15:06:04 -0700
Subject: [PATCH 10/17] libkvs/lookup: validate returned treeobj

Problem: the flux_kvs_lookup_get functions perform only
minimal validation on returned RFC 11 tree objects, which
could lead to programming errors.

Factor out decode_treeobj() from the "get" functions and
perform validation there.
---
 src/common/libkvs/kvs_lookup.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c
index 7325521bad24..b8652b8f68b1 100644
--- a/src/common/libkvs/kvs_lookup.c
+++ b/src/common/libkvs/kvs_lookup.c
@@ -173,6 +173,20 @@ static bool value_is_string (const char *s, int len)
         return true;
 }
 
+static int decode_treeobj (flux_future_t *f, json_t **treeobj)
+{
+    json_t *obj;
+
+    if (flux_rpc_get_unpack (f, "{s:o}", "val", &obj) < 0)
+        return -1;
+    if (treeobj_validate (obj) < 0) {
+        errno = EPROTO;
+        return -1;
+    }
+    *treeobj = obj;
+    return 0;
+}
+
 int flux_kvs_lookup_get (flux_future_t *f, const char **value)
 {
     struct lookup_ctx *ctx;
@@ -182,7 +196,7 @@ int flux_kvs_lookup_get (flux_future_t *f, const char **value)
         return -1;
     }
     if (!(ctx->treeobj)) {
-        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+        if (decode_treeobj (f, &ctx->treeobj) < 0)
             return -1;
     }
     if (!ctx->val_valid) {
@@ -209,7 +223,7 @@ int flux_kvs_lookup_get_treeobj (flux_future_t *f, const char **treeobj)
         return -1;
     }
     if (!(ctx->treeobj)) {
-        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+        if (decode_treeobj (f, &ctx->treeobj) < 0)
             return -1;
     }
     if (!ctx->treeobj_str) {
@@ -232,7 +246,7 @@ int flux_kvs_lookup_get_unpack (flux_future_t *f, const char *fmt, ...)
         return -1;
     }
     if (!(ctx->treeobj)) {
-        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+        if (decode_treeobj (f, &ctx->treeobj) < 0)
             return -1;
     }
     if (!ctx->val_valid) {
@@ -269,10 +283,8 @@ int flux_kvs_lookup_get_raw (flux_future_t *f, const void **data, int *len)
         return -1;
     }
     if (!(ctx->treeobj)) {
-        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0) {
-            errno = EINVAL;
+        if (decode_treeobj (f, &ctx->treeobj) < 0)
             return -1;
-        }
     }
     if (!ctx->val_valid) {
         if (treeobj_decode_val (ctx->treeobj, &ctx->val_data,
@@ -296,7 +308,7 @@ int flux_kvs_lookup_get_dir (flux_future_t *f, const flux_kvsdir_t **dirp)
         return -1;
     }
     if (!(ctx->treeobj)) {
-        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+        if (decode_treeobj (f, &ctx->treeobj) < 0)
             return -1;
     }
     if (!ctx->dir) {
@@ -320,7 +332,7 @@ int flux_kvs_lookup_get_symlink (flux_future_t *f, const char **target)
         return -1;
     }
     if (!(ctx->treeobj)) {
-        if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0)
+        if (decode_treeobj (f, &ctx->treeobj) < 0)
             return -1;
     }
     if (!treeobj_is_symlink (ctx->treeobj)) {

From 1579561022a508efb2b71d39d27c48407de04944 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 15:31:43 -0700
Subject: [PATCH 11/17] libkvs/txn: [cleanup] avoid extra encode/decode

Problem: each time an operation is appended to the transaction,
it is encoded then validated, which requires it to be decoded
first.

Move validation logic to txn_encode_op(), before encoding.

Also rename internal static function flux_kvs_txn_put_treeobj()
to append_op_to_txn().
---
 src/common/libkvs/kvs_txn.c | 41 +++++++++----------------------------
 1 file changed, 10 insertions(+), 31 deletions(-)

diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c
index 330e57185218..d8e5bc4736aa 100644
--- a/src/common/libkvs/kvs_txn.c
+++ b/src/common/libkvs/kvs_txn.c
@@ -96,39 +96,17 @@ static int validate_flags (int flags, int allowed)
     return 0;
 }
 
-static int validate_op (json_t *op)
-{
-    const char *key;
-    int flags;
-    json_t *dirent;
-
-    if (txn_decode_op (op, &key, &flags, &dirent) < 0)
-        goto error;
-    if (strlen (key) == 0)
-        goto error;
-    if (flags != 0)
-        goto error;
-    if (!json_is_null (dirent) && treeobj_validate (dirent) < 0)
-        goto error;
-    return 0;
-error:
-    errno = EINVAL;
-    return -1;
-}
-
 /* Add an operation on dirent to the transaction.
  * Takes a reference on dirent so caller retains ownership.
  */
-static int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags,
-                                     const char *key, json_t *dirent)
+static int append_op_to_txn (flux_kvs_txn_t *txn, int flags,
+                             const char *key, json_t *dirent)
 {
     json_t *op = NULL;
     int saved_errno;
 
     if (txn_encode_op (key, flags, dirent, &op) < 0)
         goto error;
-    if (validate_op (op) < 0)
-        goto error;
     if (json_array_append_new (txn->ops, op) < 0) {
         errno = ENOMEM;
         goto error;
@@ -155,7 +133,7 @@ int flux_kvs_txn_put_raw (flux_kvs_txn_t *txn, int flags,
         goto error;
     if (!(dirent = treeobj_create_val (data, len)))
         goto error;
-    if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0)
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
     return 0;
@@ -206,7 +184,7 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
         if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1)))
             goto error;
     }
-    if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0)
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
     return 0;
@@ -258,7 +236,7 @@ int flux_kvs_txn_vpack (flux_kvs_txn_t *txn, int flags,
         }
         free (s);
     }
-    if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0)
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
     return 0;
@@ -300,7 +278,7 @@ int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags,
         goto error;
     if (!(dirent = treeobj_create_dir ()))
         goto error;
-    if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0)
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
     return 0;
@@ -326,7 +304,7 @@ int flux_kvs_txn_unlink (flux_kvs_txn_t *txn, int flags,
         goto error;
     if (!(dirent = json_null ()))
         goto error;
-    if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0)
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
     return 0;
@@ -351,7 +329,7 @@ int flux_kvs_txn_symlink (flux_kvs_txn_t *txn, int flags,
         goto error;
     if (!(dirent = treeobj_create_symlink (target)))
         goto error;
-    if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0)
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
     return 0;
@@ -412,7 +390,8 @@ int txn_encode_op (const char *key, int flags, json_t *dirent, json_t **opp)
 {
     json_t *op;
 
-    if (!key || !dirent) {
+    if (!key || strlen (key) == 0 || flags != 0 || !dirent
+             || (!json_is_null (dirent) && treeobj_validate (dirent) < 0)) {
         errno = EINVAL;
         return -1;
     }

From fe17f40b1607f2b47bf70ab77f5fc1d521298c1d Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 15:57:33 -0700
Subject: [PATCH 12/17] libkvs/txn: drop special case for txn_put of NULL

If flux_kvs_txn_put() is handed a NULL value, it calls
flux_kvs_txn_unlink().  This is a bit counter-intuitive now
that, for example, a zero-length value is valid.

Drop the special case from flux_kvs_txn_put(), and make
a NULL value an EINVAL error in this commit.

Add the special case into the deprecated functions
flux_kvs_put() and flux_kvsdir_put() so that lua bindings
continue to work.

Update tests.
---
 src/common/libkvs/kvs_classic.c  | 12 ++++++++++--
 src/common/libkvs/kvs_txn.c      |  3 ---
 src/common/libkvs/test/kvs_txn.c |  8 ++++++--
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/common/libkvs/kvs_classic.c b/src/common/libkvs/kvs_classic.c
index 250ce9ebb5d4..52b835a908b9 100644
--- a/src/common/libkvs/kvs_classic.c
+++ b/src/common/libkvs/kvs_classic.c
@@ -218,9 +218,14 @@ int flux_kvs_fence_anon (flux_t *h, const char *name, int nprocs, int flags)
 int flux_kvs_put (flux_t *h, const char *key, const char *json_str)
 {
     flux_kvs_txn_t *txn = get_default_txn (h);
+    int rc;
     if (!txn)
         return -1;
-    return flux_kvs_txn_put (txn, 0, key, json_str);
+    if (json_str == NULL)
+        rc = flux_kvs_txn_unlink (txn, 0, key);
+    else
+        rc = flux_kvs_txn_put (txn, 0, key, json_str);
+    return rc;
 }
 
 int flux_kvs_unlink (flux_t *h, const char *key)
@@ -287,7 +292,10 @@ int flux_kvsdir_put (const flux_kvsdir_t *dir, const char *key,
     int rc;
     if (dir_put_init (dir, key, &dp) < 0)
         return -1;
-    rc = flux_kvs_txn_put (dp.txn, 0, dp.key, json_str);
+    if (json_str)
+        rc = flux_kvs_txn_put (dp.txn, 0, dp.key, json_str);
+    else
+        rc = flux_kvs_txn_unlink (dp.txn, 0, dp.key);
     dir_put_fini (&dp);
     return rc;
 }
diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c
index d8e5bc4736aa..cf69438ab4a3 100644
--- a/src/common/libkvs/kvs_txn.c
+++ b/src/common/libkvs/kvs_txn.c
@@ -53,7 +53,6 @@
  *
  * NULL or empty values:
  * A zero length raw value is considered valid.
- * A NULL JSON string passed to flux_kvs_txn_put() is interpreted as an unlink.
  * A NULL format string passed to flux_kvs_txn_pack() is invalid.
  */
 struct flux_kvs_txn {
@@ -155,8 +154,6 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
         errno = EINVAL;
         goto error;
     }
-    if (!json_str)
-        return flux_kvs_txn_unlink (txn, flags, key);
     if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0)
         goto error;
     /* If TREEOBJ flag, decoded json_str *is* the dirent.
diff --git a/src/common/libkvs/test/kvs_txn.c b/src/common/libkvs/test/kvs_txn.c
index ace7e6e00794..f1a03c86bf81 100644
--- a/src/common/libkvs/test/kvs_txn.c
+++ b/src/common/libkvs/test/kvs_txn.c
@@ -128,9 +128,13 @@ void basic (void)
     rc = flux_kvs_txn_put (txn, 0, "d.d.d", "43");
     ok (rc == 0,
         "6: flux_kvs_txn_put(i) works");
-    rc = flux_kvs_txn_put (txn, 0, "e", NULL);
+    rc = flux_kvs_txn_unlink (txn, 0, "e");
     ok (rc == 0,
-        "7: flux_kvs_txn_put(NULL) works");
+        "7: flux_kvs_txn_unlink works");
+    errno = 0;
+    rc = flux_kvs_txn_put (txn, 0, "nerrrrb", NULL);
+    ok (rc < 0 && errno == EINVAL,
+        "error: flux_kvs_txn_put(NULL) fails with EINVAL");
     errno = 0;
     rc = flux_kvs_txn_put (txn, 0, "f", "");
     ok (rc < 0 && errno == EINVAL,

From 032e5d874612e9b6497acde06a099fda77b246a8 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 16:31:49 -0700
Subject: [PATCH 13/17] libkvs/txn: [cleanup] add flux_kvs_txn_put_treeobj

Problem: it is a bit of a kludge to have flux_kvs_txn_put()
accept the FLUX_KVS_TREEOBJ flag and then clear it so it's
not sent in the commit request.

Add flux_kvs_txn_put_treeobj(), a dedicated function to do that.

Update the flux-kvs and tests.
---
 src/cmd/flux-kvs.c          |  8 ++--
 src/common/libkvs/kvs_txn.c | 95 ++++++++++++++++++-------------------
 src/common/libkvs/kvs_txn.h |  3 ++
 t/kvs/basic.c               |  2 +-
 4 files changed, 55 insertions(+), 53 deletions(-)

diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c
index 173167bf2e96..da909524ed3d 100644
--- a/src/cmd/flux-kvs.c
+++ b/src/cmd/flux-kvs.c
@@ -1170,8 +1170,8 @@ int cmd_copy (optparse_t *p, int argc, char **argv)
         log_err_exit ("flux_kvs_lookup %s", srckey);
     if (!(txn = flux_kvs_txn_create ()))
         log_err_exit ("flux_kvs_txn_create");
-    if (flux_kvs_txn_put (txn, FLUX_KVS_TREEOBJ, dstkey, json_str) < 0)
-        log_err_exit( "flux_kvs_txn_put");
+    if (flux_kvs_txn_put_treeobj (txn, 0, dstkey, json_str) < 0)
+        log_err_exit( "flux_kvs_txn_put_treeobj");
     flux_future_destroy (f);
 
     if (!(f = flux_kvs_commit (h, 0, txn)) || flux_future_get (f, NULL) < 0)
@@ -1202,8 +1202,8 @@ int cmd_move (optparse_t *p, int argc, char **argv)
         log_err_exit ("flux_kvs_lookup %s", srckey);
     if (!(txn = flux_kvs_txn_create ()))
         log_err_exit ("flux_kvs_txn_create");
-    if (flux_kvs_txn_put (txn, FLUX_KVS_TREEOBJ, dstkey, json_str) < 0)
-        log_err_exit( "flux_kvs_txn_put");
+    if (flux_kvs_txn_put_treeobj (txn, 0, dstkey, json_str) < 0)
+        log_err_exit( "flux_kvs_txn_put_treeobj");
     if (flux_kvs_txn_unlink (txn, 0, srckey) < 0)
         log_err_exit( "flux_kvs_txn_unlink");
     flux_future_destroy (f);
diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c
index cf69438ab4a3..aa3a17bc1a01 100644
--- a/src/common/libkvs/kvs_txn.c
+++ b/src/common/libkvs/kvs_txn.c
@@ -48,8 +48,6 @@
  * JSON, and flux_kvs_txn_pack() which builds a JSON object, then encodes it.
  * The encoded JSON is converted internally to a string, that is encoded as
  * base64 and becomes the raw value (with terminating NULL).
- * An exception is if the FLUX_KVS_TREEOBJ flag is set - then the encoded
- * or built JSON object is interpreted directly as an RFC 11 tree object.
  *
  * NULL or empty values:
  * A zero length raw value is considered valid.
@@ -144,43 +142,55 @@ int flux_kvs_txn_put_raw (flux_kvs_txn_t *txn, int flags,
     return -1;
 }
 
+int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags,
+                              const char *key, const char *treeobj)
+{
+    json_t *dirent = NULL;
+    int saved_errno;
+
+    if (!txn || !key || !treeobj) {
+        errno = EINVAL;
+        goto error;
+    }
+    if (validate_flags (flags, 0) < 0)
+        goto error;
+    if (!(dirent = treeobj_decode (treeobj)))
+        goto error;
+    if (append_op_to_txn (txn, flags, key, dirent) < 0)
+        goto error;
+    json_decref (dirent);
+    return 0;
+
+error:
+    saved_errno = errno;
+    json_decref (dirent);
+    errno = saved_errno;
+    return -1;
+}
+
 int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
                       const char *key, const char *json_str)
 {
     json_t *dirent = NULL;
     int saved_errno;
 
-    if (!txn || !key) {
+    if (!txn || !key || !json_str) {
         errno = EINVAL;
         goto error;
     }
-    if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0)
+    if (validate_flags (flags, 0) < 0)
         goto error;
-    /* If TREEOBJ flag, decoded json_str *is* the dirent.
-     * Its validity will be validated by put_treeobj().
-     * Otherwise, create a 'val' dirent, treating json_str as opaque data.
+    json_t *test;
+    /* User must pass in valid json object str, otherwise they
+     * should use flux_kvs_txn_pack() or flux_kvs_txn_put_raw()
      */
-    if ((flags & FLUX_KVS_TREEOBJ)) {
-        if (!(dirent = json_loads (json_str, JSON_DECODE_ANY, NULL))) {
-            errno = EINVAL;
-            goto error;
-        }
-        flags &= ~FLUX_KVS_TREEOBJ; // don't send in commit request
-    }
-    else {
-        json_t *test;
-
-        /* User must pass in valid json object str, otherwise they
-         * should use flux_kvs_txn_pack() or flux_kvs_txn_put_raw()
-         */
-        if (!(test = json_loads (json_str, JSON_DECODE_ANY, NULL))) {
-            errno = EINVAL;
-            goto error;
-        }
-        json_decref (test);
-        if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1)))
-            goto error;
+    if (!(test = json_loads (json_str, JSON_DECODE_ANY, NULL))) {
+        errno = EINVAL;
+        goto error;
     }
+    json_decref (test);
+    if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1)))
+        goto error;
     if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
@@ -198,41 +208,30 @@ int flux_kvs_txn_vpack (flux_kvs_txn_t *txn, int flags,
 {
     json_t *val, *dirent = NULL;
     int saved_errno;
+    char *s;
 
     if (!txn || !key | !fmt) {
         errno = EINVAL;
         goto error;
     }
-    if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0)
+    if (validate_flags (flags, 0) < 0)
         goto error;
     val = json_vpack_ex (NULL, 0, fmt, ap);
     if (!val) {
         errno = EINVAL;
         goto error;
     }
-    /* If TREEOBJ flag, the json object *is* the dirent.
-     * Its validity will be validated by put_treeobj().
-     * Otherwise, create a 'val' dirent, treating encoded json object
-     * as opaque data.
-     */
-    if ((flags & FLUX_KVS_TREEOBJ)) {
-        dirent = val;
-        flags &= ~FLUX_KVS_TREEOBJ; // don't send in commit request
-    }
-    else {
-        char *s;
-        if (!(s = json_dumps (val, JSON_ENCODE_ANY))) {
-            errno = ENOMEM;
-            json_decref (val);
-            goto error;
-        }
+    if (!(s = json_dumps (val, JSON_ENCODE_ANY))) {
+        errno = ENOMEM;
         json_decref (val);
-        if (!(dirent = treeobj_create_val (s, strlen (s) + 1))) {
-            free (s);
-            goto error;
-        }
+        goto error;
+    }
+    json_decref (val);
+    if (!(dirent = treeobj_create_val (s, strlen (s) + 1))) {
         free (s);
+        goto error;
     }
+    free (s);
     if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
     json_decref (dirent);
diff --git a/src/common/libkvs/kvs_txn.h b/src/common/libkvs/kvs_txn.h
index 3fa20152e543..e1a2dc8d69b8 100644
--- a/src/common/libkvs/kvs_txn.h
+++ b/src/common/libkvs/kvs_txn.h
@@ -24,6 +24,9 @@ int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags, const char *key,
 int flux_kvs_txn_put_raw (flux_kvs_txn_t *txn, int flags,
                           const char *key, const void *data, int len);
 
+int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags,
+                              const char *key, const char *treeobj);
+
 int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags,
                         const char *key);
 
diff --git a/t/kvs/basic.c b/t/kvs/basic.c
index 04dd0b910faa..c84e1aca0343 100644
--- a/t/kvs/basic.c
+++ b/t/kvs/basic.c
@@ -455,7 +455,7 @@ void cmd_put_treeobj (flux_t *h, int argc, char **argv)
 
     if (!(txn = flux_kvs_txn_create ()))
         log_err_exit ("flux_kvs_txn_create");
-    if (flux_kvs_txn_put (txn, FLUX_KVS_TREEOBJ, key, val) < 0)
+    if (flux_kvs_txn_put_treeobj (txn, 0, key, val) < 0)
         log_err_exit ("flux_kvs_txn_put %s=%s", key, val);
     if (!(f = flux_kvs_commit (h, 0, txn)) || flux_future_get (f, NULL) < 0)
         log_err_exit ("flux_kvs_commit");

From e35ac4856d9b853e8f15b45569912b60c5a17f67 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Oct 2017 16:45:55 -0700
Subject: [PATCH 14/17] libkvs/txn: txn_put should not require JSON value

Problem: flux_kvs_txn_put() requires its string value
to be valid JSON, but KVS values are no longer required to
be JSON.

Rename parameter to 'value' and allow it to be any NULL
terminated string.

Retain the JSON requirement in the "classic" function
flux_kvs_put() and flux_kvsdir_put().

Retain the "flux-kvs put" behavior of encoding values as
a JSON string if they aren't already valid encoded JSON.
---
 src/cmd/flux-kvs.c               |  8 ++++++--
 src/common/libkvs/kvs_classic.c  | 26 ++++++++++++++++++++++----
 src/common/libkvs/kvs_txn.c      | 22 ++++------------------
 src/common/libkvs/kvs_txn.h      |  2 +-
 src/common/libkvs/test/kvs_txn.c |  4 ----
 5 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c
index da909524ed3d..ba0ad6265623 100644
--- a/src/cmd/flux-kvs.c
+++ b/src/cmd/flux-kvs.c
@@ -380,9 +380,13 @@ int cmd_put (optparse_t *p, int argc, char **argv)
             log_msg_exit ("put: you must specify a value as key=value");
         *val++ = '\0';
 
-        if (flux_kvs_txn_put (txn, 0, key, val) < 0) {
-            if (errno != EINVAL)
+        json_t *obj;
+        if ((obj = json_loads (val, JSON_DECODE_ANY, NULL))) {
+            if (flux_kvs_txn_put (txn, 0, key, val) < 0)
                 log_err_exit ("%s", key);
+            json_decref (obj);
+        }
+        else { // encode as JSON string if not already valid encoded JSON
             if (flux_kvs_txn_pack (txn, 0, key, "s", val) < 0)
                 log_err_exit ("%s", key);
         }
diff --git a/src/common/libkvs/kvs_classic.c b/src/common/libkvs/kvs_classic.c
index 52b835a908b9..aea3c5868249 100644
--- a/src/common/libkvs/kvs_classic.c
+++ b/src/common/libkvs/kvs_classic.c
@@ -29,6 +29,7 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <czmq.h>
+#include <jansson.h>
 #include <flux/core.h>
 
 int flux_kvs_get (flux_t *h, const char *key, char **valp)
@@ -215,16 +216,30 @@ int flux_kvs_fence_anon (flux_t *h, const char *name, int nprocs, int flags)
     return rc;
 }
 
+static int validate_json (const char *json_str)
+{
+    json_t *test;
+    if (!(test = json_loads (json_str, JSON_DECODE_ANY, NULL))) {
+        errno = EINVAL;
+        return -1;
+    }
+    json_decref (test);
+    return 0;
+}
+
 int flux_kvs_put (flux_t *h, const char *key, const char *json_str)
 {
     flux_kvs_txn_t *txn = get_default_txn (h);
     int rc;
     if (!txn)
         return -1;
-    if (json_str == NULL)
-        rc = flux_kvs_txn_unlink (txn, 0, key);
-    else
+    if (json_str) {
+        if (validate_json (json_str) < 0)
+            return -1;
         rc = flux_kvs_txn_put (txn, 0, key, json_str);
+    }
+    else
+        rc = flux_kvs_txn_unlink (txn, 0, key);
     return rc;
 }
 
@@ -292,8 +307,11 @@ int flux_kvsdir_put (const flux_kvsdir_t *dir, const char *key,
     int rc;
     if (dir_put_init (dir, key, &dp) < 0)
         return -1;
-    if (json_str)
+    if (json_str) {
+        if (validate_json (json_str) < 0)
+            return -1;
         rc = flux_kvs_txn_put (dp.txn, 0, dp.key, json_str);
+    }
     else
         rc = flux_kvs_txn_unlink (dp.txn, 0, dp.key);
     dir_put_fini (&dp);
diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c
index aa3a17bc1a01..e31d7421efb8 100644
--- a/src/common/libkvs/kvs_txn.c
+++ b/src/common/libkvs/kvs_txn.c
@@ -42,12 +42,7 @@
  * Raw versus JSON values:
  * All values are base64 encoded per RFC 11, even values that
  * are themselves JSON.  This is a change from the original design,
- * which stored only JSON values.  The new design stores only untyped
- * opaque data.  Because of this history, and because it offers convenience
- * for KVS use cases, we still have flux_kvs_txn_put() which accepts encoded
- * JSON, and flux_kvs_txn_pack() which builds a JSON object, then encodes it.
- * The encoded JSON is converted internally to a string, that is encoded as
- * base64 and becomes the raw value (with terminating NULL).
+ * which stored only JSON values.
  *
  * NULL or empty values:
  * A zero length raw value is considered valid.
@@ -169,27 +164,18 @@ int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags,
 }
 
 int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
-                      const char *key, const char *json_str)
+                      const char *key, const char *value)
 {
     json_t *dirent = NULL;
     int saved_errno;
 
-    if (!txn || !key || !json_str) {
+    if (!txn || !key || !value) {
         errno = EINVAL;
         goto error;
     }
     if (validate_flags (flags, 0) < 0)
         goto error;
-    json_t *test;
-    /* User must pass in valid json object str, otherwise they
-     * should use flux_kvs_txn_pack() or flux_kvs_txn_put_raw()
-     */
-    if (!(test = json_loads (json_str, JSON_DECODE_ANY, NULL))) {
-        errno = EINVAL;
-        goto error;
-    }
-    json_decref (test);
-    if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1)))
+    if (!(dirent = treeobj_create_val (value, strlen (value) + 1)))
         goto error;
     if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
diff --git a/src/common/libkvs/kvs_txn.h b/src/common/libkvs/kvs_txn.h
index e1a2dc8d69b8..d952753c19e9 100644
--- a/src/common/libkvs/kvs_txn.h
+++ b/src/common/libkvs/kvs_txn.h
@@ -13,7 +13,7 @@ flux_kvs_txn_t *flux_kvs_txn_create (void);
 void flux_kvs_txn_destroy (flux_kvs_txn_t *txn);
 
 int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
-                      const char *key, const char *json_str);
+                      const char *key, const char *value);
 
 /* N.B. splitting at 80 columns confuses python cffi parser */
 int flux_kvs_txn_vpack (flux_kvs_txn_t *txn, int flags, const char *key, const char *fmt, va_list ap);
diff --git a/src/common/libkvs/test/kvs_txn.c b/src/common/libkvs/test/kvs_txn.c
index f1a03c86bf81..7811bb501870 100644
--- a/src/common/libkvs/test/kvs_txn.c
+++ b/src/common/libkvs/test/kvs_txn.c
@@ -136,10 +136,6 @@ void basic (void)
     ok (rc < 0 && errno == EINVAL,
         "error: flux_kvs_txn_put(NULL) fails with EINVAL");
     errno = 0;
-    rc = flux_kvs_txn_put (txn, 0, "f", "");
-    ok (rc < 0 && errno == EINVAL,
-        "error: flux_kvs_txn_put(empty string) fails with EINVAL");
-    errno = 0;
     rc = flux_kvs_txn_pack (txn, 0xFFFF, "foo.bar.blorp",  "s", "foo");
     ok (rc < 0 && errno == EINVAL,
         "error: flux_kvs_txn_pack(bad flags) fails with EINVAL");

From f5ed457a2087320d991ad96175bfe6c79aae7bc8 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Tue, 24 Oct 2017 19:35:36 -0700
Subject: [PATCH 15/17] libkvs/txn: txn_put accepts a NULL value

Modify flux_kvs_txn_put() to allow a NULL (zero length)
value to be committed.

Update comment about NULL handling.

Update txn unit test to verify that txn_put properly encodes
a NULL value.
---
 src/common/libkvs/kvs_txn.c      |  7 ++---
 src/common/libkvs/test/kvs_txn.c | 44 ++++++++++++++++++++++++++------
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c
index e31d7421efb8..219f4c225937 100644
--- a/src/common/libkvs/kvs_txn.c
+++ b/src/common/libkvs/kvs_txn.c
@@ -45,7 +45,8 @@
  * which stored only JSON values.
  *
  * NULL or empty values:
- * A zero length raw value is considered valid.
+ * A zero-length value may be stored in the KVS via
+ * flux_kvs_txn_put (value=NULL) or flux_kvs_txn_put_raw (data=NULL,len=0).
  * A NULL format string passed to flux_kvs_txn_pack() is invalid.
  */
 struct flux_kvs_txn {
@@ -169,13 +170,13 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
     json_t *dirent = NULL;
     int saved_errno;
 
-    if (!txn || !key || !value) {
+    if (!txn || !key) {
         errno = EINVAL;
         goto error;
     }
     if (validate_flags (flags, 0) < 0)
         goto error;
-    if (!(dirent = treeobj_create_val (value, strlen (value) + 1)))
+    if (!(dirent = treeobj_create_val (value, value ? strlen (value) + 1 : 0)))
         goto error;
     if (append_op_to_txn (txn, flags, key, dirent) < 0)
         goto error;
diff --git a/src/common/libkvs/test/kvs_txn.c b/src/common/libkvs/test/kvs_txn.c
index 7811bb501870..a6c535bf11c6 100644
--- a/src/common/libkvs/test/kvs_txn.c
+++ b/src/common/libkvs/test/kvs_txn.c
@@ -19,6 +19,25 @@ void jdiag (json_t *o)
     free (tmp);
 }
 
+
+/* Decode a 'val' containing base64 encoded emptiness.
+ */
+int check_null_value (json_t *dirent)
+{
+    char *data = "";
+    int len = -1;
+
+    if (treeobj_decode_val (dirent, (void **)&data, &len) < 0) {
+        diag ("%s: initial base64 decode failed", __FUNCTION__);
+        return -1;
+    }
+    if (len != 0 || data != NULL) {
+        diag ("%s: len=%d data=%p", __FUNCTION__, len, data);
+        return -1;
+    }
+    return 0;
+}
+
 /* Decode a 'val' containing base64 encoded JSON.
  * Extract a number, and compare it to 'expected'.
  */
@@ -131,10 +150,9 @@ void basic (void)
     rc = flux_kvs_txn_unlink (txn, 0, "e");
     ok (rc == 0,
         "7: flux_kvs_txn_unlink works");
-    errno = 0;
     rc = flux_kvs_txn_put (txn, 0, "nerrrrb", NULL);
-    ok (rc < 0 && errno == EINVAL,
-        "error: flux_kvs_txn_put(NULL) fails with EINVAL");
+    ok (rc == 0,
+        "8: flux_kvs_txn_put(NULL) works");
     errno = 0;
     rc = flux_kvs_txn_pack (txn, 0xFFFF, "foo.bar.blorp",  "s", "foo");
     ok (rc < 0 && errno == EINVAL,
@@ -150,8 +168,8 @@ void basic (void)
 
     /* Verify transaction contents
      */
-    ok (txn_get_op_count (txn) == 7,
-        "txn contains 7 ops");
+    ok (txn_get_op_count (txn) == 8,
+        "txn contains 8 ops");
     ok (txn_get_op (txn, 0, &entry) == 0
         && entry != NULL,
         "1: retrieved");
@@ -217,15 +235,25 @@ void basic (void)
         "7: retrieved");
     jdiag (entry);
     ok (txn_decode_op (entry, &key, &flags, &dirent) == 0,
-        "6: txn_decode_op works");
+        "7: txn_decode_op works");
     ok (!strcmp (key, "e")
         && flags == 0
         && json_is_null (dirent),
         "7: unlink e");
 
+    ok (txn_get_op (txn, 7, &entry) == 0 && entry != NULL,
+        "8: retrieved");
+    jdiag (entry);
+    ok (txn_decode_op (entry, &key, &flags, &dirent) == 0,
+        "8: txn_decode_op works");
+    ok (!strcmp (key, "nerrrrb")
+        && flags == 0
+        && check_null_value (dirent) == 0,
+        "8: put nerrrrb = NULL");
+
     errno = 0;
-    ok (txn_get_op (txn, 7, &entry) < 0 && errno == EINVAL,
-        "8: invalid (end of transaction)");
+    ok (txn_get_op (txn, 8, &entry) < 0 && errno == EINVAL,
+        "9: invalid (end of transaction)");
 
     flux_kvs_txn_destroy (txn);
 }

From 110b79c99a847dc38ebb980210553cdb2b481d72 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Tue, 24 Oct 2017 13:29:04 -0700
Subject: [PATCH 16/17] doc/flux_kvs_lookup(3): add get_symlink, get_treeobj

Add entries for the new functions, flux_kvs_lookup_get_symlink()
and flux_kvs_lookup_get_treeobj().

Split function prototypes over two lines where they didn't fit
comfortably within 80 columns in the rendered text.

Rewrite and refactor descriptions to be more clear.
---
 doc/man3/Makefile.am          |  4 ++
 doc/man3/flux_kvs_lookup.adoc | 96 +++++++++++++++++++++++------------
 doc/test/spell.en.pws         |  2 +
 3 files changed, 69 insertions(+), 33 deletions(-)

diff --git a/doc/man3/Makefile.am b/doc/man3/Makefile.am
index f65cd3f31b08..8394427adc1e 100644
--- a/doc/man3/Makefile.am
+++ b/doc/man3/Makefile.am
@@ -128,6 +128,8 @@ MAN3_FILES_SECONDARY = \
 	flux_kvs_lookup_get_unpack.3 \
 	flux_kvs_lookup_get_raw.3 \
 	flux_kvs_lookup_get_dir.3 \
+	flux_kvs_lookup_get_treeobj.3 \
+	flux_kvs_lookup_get_symlink.3 \
 	flux_kvs_fence.3 \
 	flux_kvs_txn_destroy.3 \
 	flux_kvs_txn_put.3 \
@@ -239,6 +241,8 @@ flux_kvs_lookup_get.3: flux_kvs_lookup.3
 flux_kvs_lookup_get_unpack.3: flux_kvs_lookup.3
 flux_kvs_lookup_get_raw.3: flux_kvs_lookup.3
 flux_kvs_lookup_get_dir.3: flux_kvs_lookup.3
+flux_kvs_lookup_treeobj.3: flux_kvs_lookup.3
+flux_kvs_lookup_symlink.3: flux_kvs_lookup.3
 flux_kvs_fence.3: flux_kvs_commit.3
 flux_kvs_txn_destroy.3: flux_kvs_txn_create.3
 flux_kvs_txn_put.3: flux_kvs_txn_create.3
diff --git a/doc/man3/flux_kvs_lookup.adoc b/doc/man3/flux_kvs_lookup.adoc
index 9194fe7fc97a..dd8bee9817b5 100644
--- a/doc/man3/flux_kvs_lookup.adoc
+++ b/doc/man3/flux_kvs_lookup.adoc
@@ -5,7 +5,7 @@ flux_kvs_lookup(3)
 
 NAME
 ----
-flux_kvs_lookup, flux_kvs_lookupat, flux_kvs_lookup_get, flux_kvs_lookup_get_unpack, flux_kvs_lookup_get_raw, flux_kvs_lookup_get_dir - look up KVS key
+flux_kvs_lookup, flux_kvs_lookupat, flux_kvs_lookup_get, flux_kvs_lookup_get_unpack, flux_kvs_lookup_get_raw, flux_kvs_lookup_get_dir, flux_kvs_lookup_get_treeobj, flux_kvs_lookup_get_symlink - look up KVS key
 
 
 SYNOPSIS
@@ -17,49 +17,72 @@ SYNOPSIS
  flux_future_t *flux_kvs_lookupat (flux_t *h, int flags,
                                    const char *key, const char *treeobj);
 
- int flux_kvs_lookup_get (flux_future_t *f, const char **json_str);
+ int flux_kvs_lookup_get (flux_future_t *f, const char **value);
 
  int flux_kvs_lookup_get_unpack (flux_future_t *f, const char *fmt, ...);
 
- int flux_kvs_lookup_get_raw (flux_future_t *f, const void **data, int *len);
+ int flux_kvs_lookup_get_raw (flux_future_t *f,
+                              const void **data, int *len);
 
- int flux_kvs_lookup_get_dir (flux_future_t *f, const flux_kvsdir_t **dir);
+ int flux_kvs_lookup_get_dir (flux_future_t *f,
+                              const flux_kvsdir_t **dir);
+
+ int flux_kvs_lookup_get_treeobj (flux_future_t *f, const char **treeobj);
+
+ int flux_kvs_lookup_get_symlink (flux_future_t *f, const char **target);
 
 
 DESCRIPTION
 -----------
 
 The Flux Key Value Store is a general purpose distributed storage
-service used by Flux services.  It is (currently) a single namespace
-per Flux instance and is available globally, with "loosely consistent"
-semantics.
+service used by Flux services.
 
-`flux_kvs_lookup()` sends a request to the KVS service to translate
-_key_ to its current value.  It returns a `flux_future_t` object which
-acts as handle for synchronization and container for the response.
-_flags_ modifies the request as described below.
+`flux_kvs_lookup()` sends a request to the KVS service to look up _key_.
+It returns a `flux_future_t` object which acts as handle for synchronization
+and container for the result.  _flags_ modifies the request as described
+below.
 
 `flux_kvs_lookupat()` is identical to `flux_kvs_lookup()` except
-_treeobj_ is a serialized JSON treeobj object that references a
-particular snapshot within the KVS.
+_treeobj_ is a serialized RFC 11 object that references a particular
+static set of content within the KVS, effectively a snapshot.
+See `flux_kvs_lookup_get_treeobj()` below.
+
+All the functions below are variations on a common theme.  First they
+complete the lookup RPC by blocking on the response, if not already received.
+Then they interpret the result in different ways.  They may be called more
+than once on the same future, and they may be intermixed to probe a result
+or interpret it in different ways.  Results remain valid until
+`flux_future_destroy()` is called.
+
+`flux_kvs_lookup_get()` interprets the result as a NULL-terminated string
+value.  The value is assigned to _value_.  If the value is empty, NULL
+is assigned.
+
+`flux_kvs_lookup_get_unpack()` interprets the result as a NULL-terminated
+string value, then as encoded JSON (not necessarily enclosed in an object).
+The value is parsed according to variable arguments in Jansson `json_unpack()`
+format.
 
-`flux_kvs_lookup_get ()` completes a lookup operation, blocking on
-response(s) if needed, parsing the result, and returning the requested
-value in _json_str_.  _buf_ is valid until `flux_future_destroy()` is called.
-_json_str_ may be a JSON object, array, or bare value.
+`flux_kvs_lookup_get_raw()` interprets the result as a raw, opaque value,
+which it assigns to _buf_ and its length to _len_.  The value may be any
+byte sequence.  If the value has zero length, NULL is assigned to _buf_.
 
-`flux_kvs_lookup_get_unpack()` is identical to `flux_kvs_lookup_get()` except
-the returned JSON is parsed according to variable arguments in Jansson
-`json_unpack()` format.
+`flux_kvs_lookup_get_dir()` interprets the result as a directory,
+e.g. in response to a lookup with the FLUX_KVS_READDIR flag set.
+The directory object is assigned to _dir_.
 
-`flux_kvs_lookup_get_raw()` is identical to `flux_kvs_lookup_get()` except
-the raw value is returned without decoding.
+`flux_kvs_lookup_get_treeobj()` interprets the result as any RFC 11 object.
+The object in JSON-encoded form is assigned to _treeobj_.  Since all
+lookup requests return an RFC 11 object of one type or another, this
+function should work on all.
 
-`flux_kvs_lookup_get_dir()` is identical to `flux_kvs_lookup_get()` except
-a directory object is returned.
+`flux_kvs_lookup_get_symlink()` interprets the result as a symlink target,
+e.g. in response to a lookup with the FLUX_KVS_READLINK flag set.
+The result is parsed and symlink target is assigned to _target_.
 
-These functions may be used asynchronously.
-See `flux_future_then(3)` for details.
+These functions may be used asynchronously.  See `flux_future_then(3)` for
+details.
 
 
 FLAGS
@@ -73,10 +96,15 @@ Look up a directory, not a value.  The lookup fails if the key does
 not refer to a directory object.
 
 FLUX_KVS_READLINK::
-If key is a symlink, read the link value, what it refers to.
+If key is a symlink, read the link value.  The lookup fails if the key
+does not refer to a symlink object.
 
 FLUX_KVS_TREEOBJ::
-Return the object representation.
+All KVS lookups return an RFC 11 tree object.  This flag requests that
+they be returned without conversion.  That is, a "valref" will not
+be converted to a "val" object, and a "dirref" will not be converted
+to a "dir" object.  This is useful for obtaining a snapshot reference
+that can be passed to `flux_kvs_lookupat()`.
 
 
 RETURN VALUE
@@ -85,16 +113,18 @@ RETURN VALUE
 `flux_kvs_lookup()` and `flux_kvs_lookupat()` return a
 `flux_future_t` on success, or NULL on failure with errno set appropriately.
 
-`flux_kvs_lookup_get()`, `flux_kvs_lookup_get_unpack()`, and
-`flux_kvs_lookup_get_raw()` return 0 on success, or -1 on failure with
-errno set appropriately.
+`flux_kvs_lookup_get()`, `flux_kvs_lookup_get_unpack()`,
+`flux_kvs_lookup_get_raw()`, `flux_kvs_lookup_get_dir()`,
+`flux_kvs_lookup_get_treeobj()`, and `flux_kvs_lookup_get_symlink()`
+return 0 on success, or -1 on failure with errno set appropriately.
 
 
 ERRORS
 ------
 
 EINVAL::
-One of the arguments was invalid.
+One of the arguments was invalid, or FLUX_KVS_READLINK was used but
+the key does not refer to a symlink.
 
 ENOMEM::
 Out of memory.
@@ -109,7 +139,7 @@ EISDIR::
 FLUX_KVS_READDIR flag was NOT set and key points to a directory.
 
 EPROTO::
-A request was malformed.
+A request or response was malformed.
 
 EFBIG::
 
diff --git a/doc/test/spell.en.pws b/doc/test/spell.en.pws
index a64da48e4b43..0a6764b9c252 100644
--- a/doc/test/spell.en.pws
+++ b/doc/test/spell.en.pws
@@ -402,3 +402,5 @@ procs
 txn
 kvsdir
 vpack
+dirref
+lookups

From 29e8f6e6bd5aeb2751b24b0e4003f1c22c9498db Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Tue, 24 Oct 2017 13:49:54 -0700
Subject: [PATCH 17/17] doc/flux_kvs_txn_create(3): add put_treeobj()

Add entry for flux_kvs_txn_put_treeobj().

Note that flags must always be zero now.

Rewrite and refactor descriptions to be more clear.
---
 doc/man3/Makefile.am              |  4 ++-
 doc/man3/flux_kvs_txn_create.adoc | 49 ++++++++++++++-----------------
 2 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/doc/man3/Makefile.am b/doc/man3/Makefile.am
index 8394427adc1e..93cce35dd7ad 100644
--- a/doc/man3/Makefile.am
+++ b/doc/man3/Makefile.am
@@ -138,7 +138,8 @@ MAN3_FILES_SECONDARY = \
 	flux_kvs_txn_mkdir.3 \
 	flux_kvs_txn_unlink.3 \
 	flux_kvs_txn_symlink.3 \
-	flux_kvs_txn_put_raw.3
+	flux_kvs_txn_put_raw.3 \
+	flux_kvs_txn_put_treeobj.3
 
 ADOC_FILES  = $(MAN3_FILES_PRIMARY:%.3=%.adoc)
 XML_FILES   = $(MAN3_FILES_PRIMARY:%.3=%.xml)
@@ -252,6 +253,7 @@ flux_kvs_txn_mkdir.3: flux_kvs_txn_create.3
 flux_kvs_txn_unlink.3: flux_kvs_txn_create.3
 flux_kvs_txn_symlink.3: flux_kvs_txn_create.3
 flux_kvs_txn_put_raw.3: flux_kvs_txn_create.3
+flux_kvs_txn_put_treeobj.3: flux_kvs_txn_create.3
 
 flux_open.3: topen.c
 flux_send.3: tsend.c
diff --git a/doc/man3/flux_kvs_txn_create.adoc b/doc/man3/flux_kvs_txn_create.adoc
index 99eb313da8be..588332e7d338 100644
--- a/doc/man3/flux_kvs_txn_create.adoc
+++ b/doc/man3/flux_kvs_txn_create.adoc
@@ -5,7 +5,7 @@ flux_kvs_txn_create(3)
 
 NAME
 ----
-flux_kvs_txn_create, flux_kvs_txn_destroy, flux_kvs_txn_put, flux_kvs_txn_pack, flux_kvs_txn_vpack, flux_kvs_txn_mkdir, flux_kvs_txn_unlink, flux_kvs_txn_symlink, flux_kvs_txn_put_raw - operate on a KVS transaction object
+flux_kvs_txn_create, flux_kvs_txn_destroy, flux_kvs_txn_put, flux_kvs_txn_pack, flux_kvs_txn_vpack, flux_kvs_txn_mkdir, flux_kvs_txn_unlink, flux_kvs_txn_symlink, flux_kvs_txn_put_raw, flux_kvs_txn_put_treeobj - operate on a KVS transaction object
 
 
 SYNOPSIS
@@ -17,7 +17,7 @@ SYNOPSIS
  void flux_kvs_txn_destroy (flux_kvs_txn_t *txn);
 
  int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags,
-                       const char *key, const char *json_str);
+                       const char *key, const char *value);
 
  int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags,
                         const char *key, const char *fmt, ...);
@@ -37,33 +37,33 @@ SYNOPSIS
  int flux_kvs_txn_put_raw (flux_kvs_txn_t *txn, int flags,
                            const char *key, const void *data, int len);
 
+ int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags,
+                               const char *key, const char *treeobj);
 
 
 DESCRIPTION
 -----------
 
+The Flux Key Value Store is a general purpose distributed storage
+service used by Flux services.
+
 `flux_kvs_txn_create()` creates a KVS transaction object that may be
 passed to `flux_kvs_commit(3)` or `flux_kvs_fence(3)`.  The transaction
 consists of a list of operations that are applied to the KVS together,
 in order.  The entire transaction either succeeds or fails.  After commit
 or fence, the object must be destroyed with `flux_kvs_txn_destroy()`.
 
-`flux_kvs_txn_put()` sets _key_ to a value represented by _json_str_.
-If _key_ does not exist it is created.  _key_ is hierarchical, with period
-(".") used as a path separator.  "." represents the root of the namespace
-and is optional at the beginning of _key_.  Any path components in _key_
-that do not exist is created.  Any path components in _key_ that must be
-converted to directories are overwritten.  The value _json_str_ may be be
-any bare JSON value (except null), a JSON array, or a JSON object, encoded
-as a string.  Alternatively, the FLUX_KVS_TREEOBJ flag may be specified
-indicating that the _json_str_ value is to be interpreted as an RFC 11
-tree object, as described in FLAGS below.  A NULL _json_str_ value is
-equivalent to calling `flux_kvs_txn_unlink()` on _key_.
-
-`flux_kvs_txn_pack()` is identical to `flux_kvs_txn_put()`, except
-`json_pack()` style arguments (see below) are used to construct the
-value.  `flux_kvs_txn_vpack()` is a variant that accepts a _va_list_
-argument.
+Each function below adds a single operation to _txn_.  _key_ is a
+hierarchical path name with period (".") used as path separator.
+When the transaction is committed, any existing keys or path components
+that are in conflict with the requested operation are overwritten.
+
+`flux_kvs_txn_put()` sets _key_ to a NULL terminated string _value_.
+_value_ may be NULL indicating that an empty value should be stored.
+
+`flux_kvs_txn_pack()` sets _key_ to a NULL terminated string encoded
+from a JSON object built with `json_pack()` style arguments (see below).
+`flux_kvs_txn_vpack()` is a variant that accepts a _va_list_ argument.
 
 `flux_kvs_txn_mkdir()` sets _key_ to an empty directory.
 
@@ -76,19 +76,14 @@ another key.  _target_ need not exist.
 `flux_kvs_txn_put_raw()` sets _key_ to a value containing raw data
 referred to by _data_ of length _len_.
 
+`flux_kvs_txn_put_treeobj()` sets _key_ to an RFC 11 object, encoded
+as a JSON string.
+
 
 FLAGS
 -----
 
-The _flags_ argument may be zero, or a bitmask of one or more of the
-following flags:
-
-FLUX_KVS_TREEOBJ::
-The specified value is interpreted as an RFC 11 tree object (KVS meta data)
-rather than the actual value.  Currently the only way to obtain a valid
-tree object is with `flux_kvs_lookup(3)` or `flux_kvs_lookupat(3)`.  In
-the future, other methods may be available.  Note: this flag is only
-valid for `flux_kvs_txn_put()` and `flux_kvs_txn_pack()`.
+The _flags_ argument is currently unused and must be zero.
 
 
 include::JSON_PACK.adoc[]