Fix cppcheck warnings

Lots of cppcheck fixes for new diagnostics in 1.89, plus some
additional suppressions for one disagreement and a couple of
apparent bugs.

5 files changed, 56 insertions, 41 deletions

diff --git a/tests/client/api-t.c b/tests/client/api-t.c
index a721312..977c098 100644
--- a/tests/client/api-t.c
+++ b/tests/client/api-t.c
@@ -2,6 +2,7 @@
  * Test suite for the high-level remctl library API.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
+ * Copyright 2019 Russ Allbery <eagle@eyrie.org>
  * Copyright 2006-2007, 2009-2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -59,10 +60,10 @@ test_command(struct remctl *r)
         ok_block(3, false, "...remctl_open failed");
     else {
         output = remctl_output(r);
+        ok(output != NULL, "second output token is not null");
         if (output == NULL)
-            ok_block(3, false, "second output token is not null");
+            ok_block(2, false, "second output token is not null");
         else {
-            ok(output != NULL, "second output token is not null");
             is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
             is_int(0, output->status, "...and is right status");
         }
@@ -117,10 +118,10 @@ do_tests(const char *principal, int protocol)
     ok(remctl_commandv(r, command, 2), "remctl_commandv");
     is_string("no error", remctl_error(r), "...still no error");
     output = remctl_output(r);
+    ok(output != NULL, "first output token is not null");
     if (output == NULL)
-        ok_block(5, false, "first output token is not null");
+        ok_block(4, false, "first output token is not null");
     else {
-        ok(output != NULL, "first output token is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is right type");
         is_int(12, output->length, "...and is right length");
         if (output->data == NULL)
@@ -131,10 +132,10 @@ do_tests(const char *principal, int protocol)
         is_int(1, output->stream, "...and is right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "second output token is not null");
     if (output == NULL)
-        ok_block(3, false, "second output token is not null");
+        ok_block(2, false, "second output token is not null");
     else {
-        ok(output != NULL, "second output token is not null");
         is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
         is_int(0, output->status, "...and is right status");
     }
@@ -144,11 +145,11 @@ do_tests(const char *principal, int protocol)
     ok(remctl_command(r, error), "remctl_command of error command");
     is_string("no error", remctl_error(r), "...no error on send");
     output = remctl_output(r);
+    ok(output != NULL, "first output token is not null");
     if (output == NULL)
-        ok_block(protocol == 1 ? 8 : 5, false,
+        ok_block(protocol == 1 ? 7 : 4, false,
                  "first output token is not null");
     else if (protocol == 1) {
-        ok(output != NULL, "first output token is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type,
                "...and is right protocol 1 type");
         is_int(16, output->length, "...and is right length");
@@ -159,15 +160,14 @@ do_tests(const char *principal, int protocol)
                "...and has the right error message");
         is_int(1, output->stream, "...and is right stream");
         output = remctl_output(r);
+        ok(output != NULL, "second output token is not null");
         if (output == NULL)
-            ok_block(3, false, "second output token is not null");
+            ok_block(2, false, "second output token is not null");
         else {
-            ok(output != NULL, "second output token is not null");
             is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
             is_int(-1, output->status, "...and is right status");
         }
     } else {
-        ok(output != NULL, "first output token is not null");
         is_int(REMCTL_OUT_ERROR, output->type,
                "...and is right protocol 2 type");
         is_int(15, output->length, "...and is right length");
@@ -183,10 +183,10 @@ do_tests(const char *principal, int protocol)
     ok(remctl_command(r, no_service), "remctl_command with no service");
     is_string("no error", remctl_error(r), "...and no error");
     output = remctl_output(r);
+    ok(output != NULL, "...and non-null output token");
     if (output == NULL)
-        ok_block(5, false, "...and non-null output token");
+        ok_block(4, false, "...and non-null output token");
     else {
-        ok(output != NULL, "...and non-null output token");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...of correct type");
         is_int(12, output->length, "...and length");
         if (output->data == NULL)
@@ -196,10 +196,10 @@ do_tests(const char *principal, int protocol)
         is_int(1, output->stream, "...and stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "...and non-null second token");
     if (output == NULL)
-        ok_block(3, false, "...and non-null second token");
+        ok_block(2, false, "...and non-null second token");
     else {
-        ok(output != NULL, "...and non-null second token");
         is_int(REMCTL_OUT_STATUS, output->type, "...of right type");
         is_int(0, output->status, "...and status");
     }
diff --git a/tests/data/cppcheck.supp b/tests/data/cppcheck.supp
index 33ee332..c02affe 100644
--- a/tests/data/cppcheck.supp
+++ b/tests/data/cppcheck.supp
@@ -21,6 +21,10 @@
 // every if and loop body with declarations.
 variableScope
 
+// strlen of a constant string is more maintainable code than hard-coding the
+// string length.
+constArgument:tests/runtests.c:804
+
 // False positive due to recursive function.
 knownConditionTrueFalse:portable/getopt.c:146
 
@@ -31,8 +35,18 @@ knownConditionTrueFalse:tests/tap/remctl.c:79
 // because of the struct sockaddr * cast.
 memleak:portable/getaddrinfo.c:236
 
+// Bug in cppcheck 1.89.  The address of this variable is passed to a Windows
+// function (albeit through a cast).
+nullPointer:portable/winsock.c:62
+
 // Setting the variable to NULL explicitly after deallocation.
 redundantAssignment:tests/pam-util/options-t.c
 
-// (remctl) Nested assignments in this test confuse cppcheck.
-redundantAssignment:tests/server/acl-t.c
+// (remctl) Bug in cppcheck 1.89.  The address of these variables are passed
+// to a PHP function.
+uninitvar:php/php_remctl.c:119
+uninitvar:php/php_remctl.c:123
+uninitvar:php/php_remctl.c:315
+uninitvar:php/php5_remctl.c:125
+uninitvar:php/php5_remctl.c:129
+uninitvar:php/php5_remctl.c:321
diff --git a/tests/server/continue-t.c b/tests/server/continue-t.c
index 83652e8..3863ee4 100644
--- a/tests/server/continue-t.c
+++ b/tests/server/continue-t.c
@@ -2,7 +2,7 @@
  * Test suite for continued commands.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2018 Russ Allbery <eagle@eyrie.org>
+ * Copyright 2018-2019 Russ Allbery <eagle@eyrie.org>
  * Copyright 2006, 2009-2010, 2012-2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -88,10 +88,10 @@ main(void)
     is_int(TOKEN_OK, status, "fourth token sent okay");
     r->ready = 1;
     output = remctl_output(r);
+    ok(output != NULL, "got output");
     if (output == NULL)
-        ok_block(3, false, "got output");
+        ok_block(2, false, "got output");
     else {
-        ok(output != NULL, "got output");
         is_int(REMCTL_OUT_STATUS, output->type, "...of type status");
         is_int(2, output->status, "...with correct status");
     }
diff --git a/tests/server/stdin-t.c b/tests/server/stdin-t.c
index 7fcff2d..b62448c 100644
--- a/tests/server/stdin-t.c
+++ b/tests/server/stdin-t.c
@@ -2,7 +2,7 @@
  * Test suite for the server passing data to programs on standard input.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
- * Copyright 2018 Russ Allbery <eagle@eyrie.org>
+ * Copyright 2018-2019 Russ Allbery <eagle@eyrie.org>
  * Copyright 2009-2010, 2012-2013
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -48,10 +48,10 @@ test_stdin(const char *principal, const char *test, const void *data,
         bail("can't connect: %s", remctl_error(r));
     ok(remctl_commandv(r, command, 4), "sent command for %s", test);
     output = remctl_output(r);
+    ok(output != NULL, "first output token is not null");
     if (output == NULL)
-        ok_block(5, false, "first output token is not null");
+        ok_block(4, false, "first output token is not null");
     else {
-        ok(output != NULL, "first output token is not null");
         diag("status: %d", output->status);
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is right type");
         is_int(strlen("Okay"), output->length, "...and is right length");
@@ -64,10 +64,10 @@ test_stdin(const char *principal, const char *test, const void *data,
         is_int(1, output->stream, "...and is right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "second output token is not null");
     if (output == NULL)
-        ok_block(3, false, "second output token is not null");
+        ok_block(2, false, "second output token is not null");
     else {
-        ok(output != NULL, "second output token is not null");
         is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
         is_int(0, output->status, "...and is right status");
     }
diff --git a/tests/server/streaming-t.c b/tests/server/streaming-t.c
index 163119b..172bf6b 100644
--- a/tests/server/streaming-t.c
+++ b/tests/server/streaming-t.c
@@ -2,6 +2,7 @@
  * Test suite for streaming data from the server.
  *
  * Written by Russ Allbery <eagle@eyrie.org>
+ * Copyright 2019 Russ Allbery <eagle@eyrie.org>
  * Copyright 2006, 2009-2010, 2012-2014
  *     The Board of Trustees of the Leland Stanford Junior University
  *
@@ -41,10 +42,10 @@ main(void)
     ok(remctl_open(r, "localhost", 14373, config->principal), "remctl_open");
     ok(remctl_command(r, command_streaming), "remctl_command streaming");
     output = remctl_output(r);
+    ok(output != NULL, "output is not null");
     if (output == NULL)
-        ok_block(5, false, "output is not null");
+        ok_block(4, false, "output is not null");
     else {
-        ok(output != NULL, "output is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is correct type");
         is_int(23, output->length, "right length for first line");
         if (output->data == NULL)
@@ -55,10 +56,10 @@ main(void)
         is_int(1, output->stream, "...right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "second output is not null");
     if (output == NULL)
-        ok_block(5, false, "second output is not null");
+        ok_block(4, false, "second output is not null");
     else {
-        ok(output != NULL, "second output is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is correct type");
         is_int(24, output->length, "right length for second line");
         if (output->data == NULL)
@@ -69,10 +70,10 @@ main(void)
         is_int(2, output->stream, "...right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "third output is not null");
     if (output == NULL)
-        ok_block(5, false, "third output is not null");
+        ok_block(4, false, "third output is not null");
     else {
-        ok(output != NULL, "third output is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is correct type");
         is_int(23, output->length, "right length for third line");
         if (output->data == NULL)
@@ -83,10 +84,10 @@ main(void)
         is_int(1, output->stream, "...right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "status is not null");
     if (output == NULL)
-        ok_block(3, false, "status is not null");
+        ok_block(2, false, "status is not null");
     else {
-        ok(output != NULL, "status is not null");
         is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
         is_int(0, output->status, "...and is right status");
     }
@@ -126,10 +127,10 @@ main(void)
     ok(remctl_open(r, "localhost", 14373, config->principal), "remctl_open");
     ok(remctl_command(r, command_streaming), "remctl_command");
     output = remctl_output(r);
+    ok(output != NULL, "output is not null");
     if (output == NULL)
-        ok_block(5, false, "output is not null");
+        ok_block(4, false, "output is not null");
     else {
-        ok(output != NULL, "output is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is right type");
         is_int(70, output->length, "...and right length");
         if (output->data == NULL)
@@ -141,10 +142,10 @@ main(void)
         is_int(1, output->stream, "...and right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "status token is not null");
     if (output == NULL)
-        ok_block(3, false, "status token is not null");
+        ok_block(2, false, "status token is not null");
     else {
-        ok(output != NULL, "status token is not null");
         is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
         is_int(0, output->status, "...and is right status");
     }
@@ -163,19 +164,19 @@ main(void)
     ok(remctl_open(r, "localhost", 14373, config->principal), "remctl_open");
     ok(remctl_command(r, command_cat), "remctl_command cat");
     output = remctl_output(r);
+    ok(output != NULL, "output is not null");
     if (output == NULL)
-        ok_block(4, false, "output is not null");
+        ok_block(3, false, "output is not null");
     else {
-        ok(output != NULL, "output is not null");
         is_int(REMCTL_OUT_OUTPUT, output->type, "...and is right type");
         is_int(TOKEN_MAX_OUTPUT_V1, output->length, "...and right length");
         is_int(1, output->stream, "...and right stream");
     }
     output = remctl_output(r);
+    ok(output != NULL, "status token is not null");
     if (output == NULL)
-        ok_block(3, false, "status token is not null");
+        ok_block(2, false, "status token is not null");
     else {
-        ok(output != NULL, "status token is not null");
         is_int(REMCTL_OUT_STATUS, output->type, "...and is right type");
         is_int(0, output->status, "...and is right status");
     }