Merge pull request #1166 from chu11/issue1157

libflux: Add input checks to future functions

doc/man3/flux_future_create.adoc

@@ -140,8 +140,8 @@ returned and errno is set appropriately.
 `flux_future_aux_set()` returns zero on success.  On error, -1 is
 returned and errno is set appropriately.
 
-`flux_future_aux_get()` returns the requested object on success.
-If it is not found NULL.  This function does not set errno.
+`flux_future_aux_get()` returns the requested object on success.  On
+error, NULL is returned and errno is set appropriately.
 
 `flux_future_get_flux()` returns a flux_t handle on success.  On error,
 NULL is returned and errno is set appropriately.
@@ -156,6 +156,9 @@ Out of memory.
 EINVAL::
 Invalid argument.
 
+ENOENT::
+The requested object is not found.
+
 AUTHOR
 ------
 This page is maintained by the Flux community.

src/common/libflux/future.c

@@ -232,7 +232,8 @@ flux_future_t *flux_future_create (flux_reactor_t *r,
  */
 void flux_future_set_flux (flux_future_t *f, flux_t *h)
 {
-    f->h = h;
+    if (f)
+        f->h = h;
 }
 
 /* Context dependent get of flux handle.
@@ -243,7 +244,7 @@ flux_t *flux_future_get_flux (flux_future_t *f)
 {
     flux_t *h = NULL;
 
-    if (!f->h) {
+    if (!f || !f->h) {
         errno = EINVAL;
         goto done;
     }
@@ -275,6 +276,10 @@ flux_t *flux_future_get_flux (flux_future_t *f)
  */
 int flux_future_wait_for (flux_future_t *f, double timeout)
 {
+    if (!f) {
+        errno = EINVAL;
+        return -1;
+    }
     if (!f->result_valid && !f->result_errnum_valid) {
         if (timeout == 0.) { // don't setup 'now' context in this case
             errno = ETIMEDOUT;
@@ -330,7 +335,7 @@ int flux_future_get (flux_future_t *f, void *result)
 int flux_future_then (flux_future_t *f, double timeout,
                       flux_continuation_f cb, void *arg)
 {
-    if (!f->r || !cb || f->then != NULL) {
+    if (!f || !f->r || !cb || f->then != NULL) {
         errno = EINVAL;
         return -1;
     }
@@ -353,9 +358,20 @@ int flux_future_then (flux_future_t *f, double timeout,
  */
 void *flux_future_aux_get (flux_future_t *f, const char *name)
 {
-    if (!f->aux)
+    void *rv;
+
+    if (!f) {
+        errno = EINVAL;
         return NULL;
-    return zhash_lookup (f->aux, name);
+    }
+    if (!f->aux) {
+        errno = ENOENT;
+        return NULL;
+    }
+    /* zhash_lookup won't set errno if not found */
+    if (!(rv = zhash_lookup (f->aux, name)))
+        errno = ENOENT;
+    return rv;
 }
 
 /* Store 'aux' object by name.  Allow "anonymous" (name=NULL) objects to
@@ -368,7 +384,7 @@ int flux_future_aux_set (flux_future_t *f, const char *name,
 {
     char name_buf[32];
 
-    if (!name && !destroy) {
+    if (!f || (!name && !destroy)) {
         errno = EINVAL;
         return -1;
     }
@@ -393,33 +409,37 @@ int flux_future_aux_set (flux_future_t *f, const char *name,
 
 void flux_future_fulfill (flux_future_t *f, void *result, flux_free_f free_fn)
 {
-    if (f->result) {
-        if (f->result_free)
-            f->result_free (f->result);
+    if (f) {
+        if (f->result) {
+            if (f->result_free)
+                f->result_free (f->result);
+        }
+        f->result = result;
+        f->result_free = free_fn;
+        f->result_valid = true;
+        now_context_clear_timer (f->now);
+        then_context_clear_timer (f->then);
+        if (f->now)
+            flux_reactor_stop (f->now->r);
+        /* in "then" context, the main reactor prepare/check/idle watchers
+         * will run continuation in the next reactor loop for fairness.
+         */
     }
-    f->result = result;
-    f->result_free = free_fn;
-    f->result_valid = true;
-    now_context_clear_timer (f->now);
-    then_context_clear_timer (f->then);
-    if (f->now)
-        flux_reactor_stop (f->now->r);
-    /* in "then" context, the main reactor prepare/check/idle watchers
-     * will run continuation in the next reactor loop for fairness.
-     */
 }
 
 void flux_future_fulfill_error (flux_future_t *f, int errnum)
 {
-    f->result_errnum = errnum;
-    f->result_errnum_valid = true;
-    now_context_clear_timer (f->now);
-    then_context_clear_timer (f->then);
-    if (f->now)
-        flux_reactor_stop (f->now->r);
-    /* in "then" context, the main reactor prepare/check/idle watchers
-     * will run continuation in the next reactor loop for fairness.
-     */
+    if (f) {
+        f->result_errnum = errnum;
+        f->result_errnum_valid = true;
+        now_context_clear_timer (f->now);
+        then_context_clear_timer (f->then);
+        if (f->now)
+            flux_reactor_stop (f->now->r);
+        /* in "then" context, the main reactor prepare/check/idle watchers
+         * will run continuation in the next reactor loop for fairness.
+         */
+    }
 }
 
 /* timer - for flux_future_then() timeout

src/common/libflux/test/future.c

@@ -50,18 +50,36 @@ void test_simple (void)
     if (!f)
         BAIL_OUT ("flux_future_create failed");
 
+    /* before aux is set */
+    errno = 0;
+    char *p = flux_future_aux_get (f, "foo");
+    ok (p == NULL
+        && errno == ENOENT,
+        "flux_future_aux_get of wrong value returns ENOENT");
+
     /* aux */
     errno = 0;
     ok (flux_future_aux_set (f, NULL, "bar", NULL) < 0
          && errno == EINVAL,
         "flux_future_aux_set anon w/o destructor is EINVAL");
+    errno = 0;
+    ok (flux_future_aux_set (NULL, "foo", "bar", aux_destroy) < 0
+         && errno == EINVAL,
+        "flux_future_aux_set w/ NULL future is EINVAL");
     aux_destroy_called = 0;
     aux_destroy_arg = NULL;
     ok (flux_future_aux_set (f, "foo", "bar", aux_destroy) == 0,
         "flux_future_aux_set works");
-    char *p = flux_future_aux_get (f, "baz");
-    ok (p == NULL,
-        "flux_future_aux_get of wrong value returns NULL");
+    errno = 0;
+    p = flux_future_aux_get (NULL, "baz");
+    ok (p == NULL
+        && errno == EINVAL,
+        "flux_future_aux_get with bad input returns EINVAL");
+    errno = 0;
+    p = flux_future_aux_get (f, "baz");
+    ok (p == NULL
+        && errno == ENOENT,
+        "flux_future_aux_get of wrong value returns ENOENT");
     p = flux_future_aux_get (f, "foo");
     ok (p != NULL && !strcmp (p, "bar"),
         "flux_future_aux_get of known returns it");
@@ -71,6 +89,9 @@ void test_simple (void)
 
     /* wait_for/get - no future_init; artificially call fulfill */
     errno = 0;
+    ok (flux_future_wait_for (NULL, 0.) < 0 && errno == EINVAL,
+        "flux_future_wait_for w/ NULL future returns EINVAL");
+    errno = 0;
     ok (flux_future_wait_for (f, 0.) < 0 && errno == ETIMEDOUT,
         "flux_future_wait_for initially times out");
     errno = 0;
@@ -87,6 +108,10 @@ void test_simple (void)
         "flux_future_get with NULL results argument also works");
 
     /* continuation (result already ready) */
+    errno = 0;
+    ok (flux_future_then (NULL, -1., contin, "nerp") < 0
+        && errno == EINVAL,
+        "flux_future_then w/ NULL future returns EINVAL");
     contin_called = 0;
     contin_arg = NULL;
     contin_get_rc = -42;

src/common/libkvs/test/kvs_lookup.c

@@ -20,6 +20,14 @@ void errors (void)
     errno = 0;
     ok (flux_kvs_lookupat (NULL, 0, NULL, NULL) == NULL && errno == EINVAL,
         "flux_kvs_lookupat fails on bad input");
+
+    errno = 0;
+    ok (flux_kvs_lookup_get (NULL, NULL) < 0 && errno == EINVAL,
+        "flux_kvs_lookup_get fails on bad input");
+
+    errno = 0;
+    ok (flux_kvs_lookup_get_unpack (NULL, NULL) < 0 && errno == EINVAL,
+        "flux_kvs_lookup_get_unpack fails on bad input");
 }
 
 int main (int argc, char *argv[])