Clean up assumptions about free(NULL) and NULL pointers

Stop checking whether something is non-NULL before freeing it.
POSIX guarantees you can just call the function without checking,
and several of our supporting data structures provide the same
guarantee.

Assume that memset/calloc will set all the pointers to NULL and
they don't have to be set explicitly.

Change-Id: Iea6529c1603aa7724647387f92fc5d4dc5dc708b
Reviewed-on: https://gerrit.stanford.edu/1395
Reviewed-by: Russ Allbery <rra@stanford.edu>
Tested-by: Russ Allbery <rra@stanford.edu>

3 files changed, 35 insertions, 69 deletions

diff --git a/client/api.c b/client/api.c
index a46de3f..c6a068c 100644
--- a/client/api.c
+++ b/client/api.c
@@ -88,16 +88,14 @@ internal_output_append(struct remctl_result *result,
         buffer = &result->stderr_buf;
         length = &result->stderr_len;
     } else if (output->type == REMCTL_OUT_OUTPUT) {
-        if (result->error != NULL)
-            free(result->error);
+        free(result->error);
         status = asprintf(&result->error, "bad output stream %d",
                           output->stream);
         if (status < 0)
             result->error = NULL;
         return false;
     } else {
-        if (result->error != NULL)
-            free(result->error);
+        free(result->error);
         result->error = strdup("internal error: bad output type");
         return false;
     }
@@ -115,8 +113,7 @@ internal_output_append(struct remctl_result *result,
         newlen++;
     newbuf = realloc(*buffer, newlen);
     if (newbuf == NULL) {
-        if (result->error != NULL)
-            free(result->error);
+        free(result->error);
         result->error = strdup("cannot allocate memory");
         return false;
     }
@@ -188,15 +185,12 @@ remctl(const char *host, unsigned short port, const char *principal,
 void
 remctl_result_free(struct remctl_result *result)
 {
-    if (result != NULL) {
-        if (result->error != NULL)
-            free(result->error);
-        if (result->stdout_buf != NULL)
-            free(result->stdout_buf);
-        if (result->stderr_buf != NULL)
-            free(result->stderr_buf);
-        free(result);
-    }
+    if (result == NULL)
+        return;
+    free(result->error);
+    free(result->stdout_buf);
+    free(result->stderr_buf);
+    free(result);
 }
 
 
@@ -216,18 +210,8 @@ remctl_new(void)
     r = calloc(1, sizeof(struct remctl));
     if (r == NULL)
         return NULL;
-    r->source = NULL;
     r->fd = INVALID_SOCKET;
-    r->host = NULL;
-    r->principal = NULL;
-    r->ccache = NULL;
     r->context = GSS_C_NO_CONTEXT;
-    r->error = NULL;
-    r->output = NULL;
-#ifdef HAVE_KRB5
-    r->krb_ctx = NULL;
-    r->krb_ccache = NULL;
-#endif
     return r;
 }
 
@@ -259,8 +243,7 @@ remctl_set_ccache(struct remctl *r, const char *ccache)
         internal_set_error(r, "cannot allocate memory: %s", strerror(errno));
         return 0;
     }
-    if (r->ccache != NULL)
-        free(r->ccache);
+    free(r->ccache);
     r->ccache = copy;
     return 1;
 }
@@ -304,8 +287,7 @@ remctl_set_source_ip(struct remctl *r, const char *source)
         internal_set_error(r, "cannot allocate memory: %s", strerror(errno));
         return 0;
     }
-    if (r->source != NULL)
-        free(r->source);
+    free(r->source);
     r->source = copy;
     return 1;
 }
@@ -336,10 +318,8 @@ internal_reset(struct remctl *r)
             internal_v2_quit(r);
         socket_close(r->fd);
     }
-    if (r->error != NULL) {
-        free(r->error);
-        r->error = NULL;
-    }
+    free(r->error);
+    r->error = NULL;
     if (r->output != NULL) {
         internal_output_wipe(r->output);
         free(r->output);
@@ -360,6 +340,7 @@ remctl_open(struct remctl *r, const char *host, unsigned short port,
     socket_type fd = INVALID_SOCKET;
     char *old_error;
 
+    /* Reset and reconfigure the client object. */
     internal_reset(r);
     r->host = host;
     r->port = port;
@@ -477,17 +458,13 @@ remctl_close(struct remctl *r)
     if (r != NULL) {
         if (r->protocol > 1 && r->fd != -1)
             internal_v2_quit(r);
-        if (r->source != NULL)
-            free(r->source);
-        if (r->ccache != NULL)
-            free(r->ccache);
-        if (r->fd != -1)
+        free(r->source);
+        free(r->ccache);
+        if (r->fd != INVALID_SOCKET)
             socket_close(r->fd);
-        if (r->error != NULL)
-            free(r->error);
+        free(r->error);
         if (r->output != NULL) {
-            if (r->output->data != NULL)
-                free(r->output->data);
+            free(r->output->data);
             free(r->output);
         }
         if (r->context != GSS_C_NO_CONTEXT)
@@ -522,10 +499,8 @@ internal_reopen(struct remctl *r)
         if (!remctl_open(r, r->host, r->port, r->principal))
             return false;
     }
-    if (r->error != NULL) {
-        free(r->error);
-        r->error = NULL;
-    }
+    free(r->error);
+    r->error = NULL;
     return true;
 }
 
@@ -610,15 +585,9 @@ internal_output_wipe(struct remctl_output *output)
 {
     if (output == NULL)
         return;
+    free(output->data);
+    memset(output, 0, sizeof(*output));
     output->type = REMCTL_OUT_DONE;
-    if (output->data != NULL) {
-        free(output->data);
-        output->data = NULL;
-    }
-    output->length = 0;
-    output->stream = 0;
-    output->status = 0;
-    output->error = 0;
 }
 
 
@@ -643,10 +612,8 @@ remctl_output(struct remctl *r)
         internal_set_error(r, "no connection open");
         return NULL;
     }
-    if (r->error != NULL) {
-        free(r->error);
-        r->error = NULL;
-    }
+    free(r->error);
+    r->error = NULL;
     if (r->protocol == 1)
         return internal_v1_output(r);
     else
diff --git a/client/error.c b/client/error.c
index 40edee6..7072456 100644
--- a/client/error.c
+++ b/client/error.c
@@ -7,7 +7,7 @@
  * return the appropriate details.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2006, 2007, 2008, 2010, 2013
+ * Copyright 2006, 2007, 2008, 2010, 2013, 2014
  *     The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
@@ -34,8 +34,7 @@ internal_set_error(struct remctl *r, const char *format, ...)
     va_list args;
     int status;
 
-    if (r->error != NULL)
-        free(r->error);
+    free(r->error);
     va_start(args, format);
     status = vasprintf(&r->error, format, args);
     va_end(args);
@@ -57,8 +56,7 @@ void
 internal_gssapi_error(struct remctl *r, const char *error, OM_uint32 major,
                       OM_uint32 minor)
 {
-    if (r->error != NULL)
-        free(r->error);
+    free(r->error);
     r->error = gssapi_error_string(error, major, minor);
 }
 
@@ -90,6 +88,8 @@ void
 internal_token_error(struct remctl *r, const char *error, int status,
                      OM_uint32 major, OM_uint32 minor)
 {
+    const char *message;
+
     switch (status) {
     case TOKEN_OK:
         internal_set_error(r, "error %s", error);
@@ -98,8 +98,8 @@ internal_token_error(struct remctl *r, const char *error, int status,
         internal_set_error(r, "error %s: %s", error, strerror(errno));
         break;
     case TOKEN_FAIL_SOCKET:
-        internal_set_error(r, "error %s: %s", error,
-                           socket_strerror(socket_errno));
+        message = socket_strerror(socket_errno);
+        internal_set_error(r, "error %s: %s", error, message);
         break;
     case TOKEN_FAIL_INVALID:
         internal_set_error(r, "error %s: invalid token format", error);
diff --git a/client/open.c b/client/open.c
index 39f2c07..31edd40 100644
--- a/client/open.c
+++ b/client/open.c
@@ -8,8 +8,8 @@
  *
  * Written by Russ Allbery <eagle@eyrie.org>
  * Based on work by Anton Ushakov
- * Copyright 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013
- *     The Board of Trustees of the Leland Stanford Junior University
+ * Copyright 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013,
+ *     2014 The Board of Trustees of the Leland Stanford Junior University
  *
  * See LICENSE for licensing terms.
  */
@@ -113,8 +113,7 @@ internal_import_name(struct remctl *r, const char *host,
     else
         oid = GSS_C_NT_HOSTBASED_SERVICE;
     major = gss_import_name(&minor, &name_buffer, oid, name);
-    if (defprinc != NULL)
-        free(defprinc);
+    free(defprinc);
     if (major != GSS_S_COMPLETE) {
         internal_gssapi_error(r, "parsing name", major, minor);
         return false;