Merge pull request #4448 from larsewi/CFE-3415

olehermanse · web-flow · commit f6e841c67d29 · 2021-01-13T04:05:25.000+01:00
CFE-3415: Fixed some sign-compare warnings
diff --git a/cf-agent/verify_files.c b/cf-agent/verify_files.c
@@ -737,7 +737,7 @@ static bool SaveBufferCallback(const char *dest_filename, void *param, NewLineMo
     if (bytes_written != BufferSize(output_buffer))
     {
         Log(LOG_LEVEL_ERR,
-            "Error writing to output file '%s' when writing. %zu bytes written but expected %u. (fclose: %s)",
+            "Error writing to output file '%s' when writing. %zu bytes written but expected %zu. (fclose: %s)",
             dest_filename, bytes_written, BufferSize(output_buffer), GetErrorStr());
         fclose(fp);
         return false;
diff --git a/cf-check/backup.c b/cf-check/backup.c
@@ -106,7 +106,7 @@ int backup_files_copy(Seq *filenames)
     Log(LOG_LEVEL_INFO, "Backing up to '%s'", backup_dir);
 
     int ret = 0;
-    for (int i = 0; i < length; ++i)
+    for (size_t i = 0; i < length; ++i)
     {
         const char *file = SeqAt(filenames, i);
         if (!File_CopyToDir(file, backup_dir))
@@ -137,7 +137,7 @@ static int backup_files_replicate(const Seq *files)
     Log(LOG_LEVEL_INFO, "Backing up to '%s' using data replication", backup_dir);
 
     size_t corrupted = 0;
-    for (int i = 0; i < length; ++i)
+    for (size_t i = 0; i < length; ++i)
     {
         const char *file = SeqAt(files, i);
         assert(StringEndsWith(backup_dir, "/"));
diff --git a/cf-check/diagnose.c b/cf-check/diagnose.c
@@ -515,7 +515,7 @@ static char *follow_symlink(const char *path)
     {
         return NULL;
     }
-    if (r >= sizeof(target_buf))
+    if ((size_t) r >= sizeof(target_buf))
     {
         Log(LOG_LEVEL_ERR, "Symlink target path too long: %s", path);
         return NULL;
@@ -548,7 +548,7 @@ size_t diagnose_files(
         *corrupt = SeqNew(length, free);
     }
 
-    for (int i = 0; i < length; ++i)
+    for (size_t i = 0; i < length; ++i)
     {
         const char *filename = SeqAt(filenames, i);
         const char *symlink = NULL; // Only initialized because of gcc warning
diff --git a/cf-check/dump.c b/cf-check/dump.c
@@ -458,7 +458,7 @@ int dump_main(int argc, const char *const *const argv)
     dump_mode mode = DUMP_NICE;
     size_t offset = 1;
 
-    if (argc > offset && argv[offset] != NULL && argv[offset][0] == '-')
+    if ((size_t) argc > offset && argv[offset] != NULL && argv[offset][0] == '-')
     {
         const char *const option = argv[offset];
         offset += 1;
@@ -491,7 +491,7 @@ int dump_main(int argc, const char *const *const argv)
         }
     }
 
-    if (argc > offset && argv[offset] != NULL && argv[offset][0] == '-')
+    if ((size_t) argc > offset && argv[offset] != NULL && argv[offset][0] == '-')
     {
         print_usage();
         printf("Only one option supported!\n");
diff --git a/cf-check/lmdump.c b/cf-check/lmdump.c
@@ -10,7 +10,7 @@
 
 static void lmdump_print_hex(const char *s, size_t len)
 {
-    for (int i = 0; i < len; i++)
+    for (size_t i = 0; i < len; i++)
     {
         printf("%02x", s[i]);
     }
diff --git a/cf-check/repair.c b/cf-check/repair.c
@@ -249,7 +249,7 @@ int repair_lmdb_files(Seq *files, bool force)
     const size_t length = SeqLength(corrupt);
     assert(length > 0);
     backup_files_copy(corrupt);
-    for (int i = 0; i < length; ++i)
+    for (size_t i = 0; i < length; ++i)
     {
         const char *file = SeqAt(corrupt, i);
         if (repair_lmdb_file(file, -1) == -1)
diff --git a/cf-check/utilities.c b/cf-check/utilities.c
@@ -22,7 +22,8 @@ Seq *default_lmdb_files()
 Seq *argv_to_lmdb_files(
     const int argc, const char *const *const argv, const size_t offset)
 {
-    if (offset >= argc)
+    assert(argc >= 0);
+    if (offset >= (size_t) argc)
     {
         Log(LOG_LEVEL_INFO,
             "No filenames specified, defaulting to .lmdb files in %s",
diff --git a/libcfnet/client_code.c b/libcfnet/client_code.c
@@ -359,7 +359,11 @@ Item *RemoteDirList(const char *dirname, bool encrypt, AgentConnection *conn)
 
         tosend = cipherlen + CF_PROTO_OFFSET;
 
-        if(tosend > sizeof(sendbuffer))
+        if (tosend < 0)
+        {
+            ProgrammingError("RemoteDirList: tosend (%d) < 0", tosend);
+        }
+        else if ((unsigned long) tosend > sizeof(sendbuffer))
         {
             ProgrammingError("RemoteDirList: tosend (%d) > sendbuffer (%zd)",
                              tosend, sizeof(sendbuffer));
@@ -495,7 +499,11 @@ bool CompareHashNet(const char *file1, const char *file2, bool encrypt, AgentCon
 
         tosend = cipherlen + CF_PROTO_OFFSET;
 
-        if(tosend > sizeof(sendbuffer))
+        if (tosend < 0)
+        {
+            ProgrammingError("CompareHashNet: tosend (%d) < 0", tosend);
+        }
+        else if ((unsigned long) tosend > sizeof(sendbuffer))
         {
             ProgrammingError("CompareHashNet: tosend (%d) > sendbuffer (%zd)",
                              tosend, sizeof(sendbuffer));
@@ -589,7 +597,11 @@ static bool EncryptCopyRegularFileNet(const char *source, const char *dest, off_
 
     tosend = cipherlen + CF_PROTO_OFFSET;
 
-    if(tosend > sizeof(workbuf))
+    if (tosend < 0)
+    {
+        ProgrammingError("EncryptCopyRegularFileNet: tosend (%d) < 0", tosend);
+    }
+    else if ((unsigned long) tosend > sizeof(workbuf))
     {
         ProgrammingError("EncryptCopyRegularFileNet: tosend (%d) > workbuf (%zd)",
                          tosend, sizeof(workbuf));
diff --git a/libcfnet/stat_cache.c b/libcfnet/stat_cache.c
@@ -165,7 +165,11 @@ int cf_remote_stat(AgentConnection *conn, bool encrypt, const char *file,
 
         tosend = cipherlen + CF_PROTO_OFFSET;
 
-        if(tosend > sizeof(sendbuffer))
+        if (tosend < 0)
+        {
+            ProgrammingError("cf_remote_stat: tosend (%d) < 0", tosend);
+        }
+        else if((unsigned int) tosend > sizeof(sendbuffer))
         {
             ProgrammingError("cf_remote_stat: tosend (%d) > sendbuffer (%zd)",
                              tosend, sizeof(sendbuffer));
diff --git a/libcfnet/tls_client.c b/libcfnet/tls_client.c
@@ -250,7 +250,12 @@ int TLSClientIdentificationDialog(ConnectionInfo *conn_info,
     {
         ret = snprintf(&line[line_len], sizeof(line) - line_len,
                        " USERNAME=%s", username);
-        if (ret >= sizeof(line) - line_len)
+        if (ret < 0)
+        {
+            Log(LOG_LEVEL_ERR, "snprintf failed: %s", GetErrorStr());
+            return -1;
+        }
+        else if ((unsigned int) ret >= sizeof(line) - line_len)
         {
             Log(LOG_LEVEL_ERR, "Sending IDENTITY truncated: %s", line);
             return -1;
@@ -275,14 +280,13 @@ int TLSClientIdentificationDialog(ConnectionInfo *conn_info,
     static const char OK[] = "OK WELCOME";
     size_t OK_len = sizeof(OK) - 1;
     ret = TLSRecvLines(conn_info->ssl, line, sizeof(line));
-    if (ret == -1)
+    if (ret < 0)
     {
         Log(LOG_LEVEL_ERR,
             "Connection was hung up during identification! (3)");
         return -1;
     }
-
-    if (ret < OK_len || strncmp(line, OK, OK_len) != 0)
+    else if ((size_t) ret < OK_len || strncmp(line, OK, OK_len) != 0)
     {
         Log(LOG_LEVEL_ERR,
             "Peer did not accept our identity! Responded: %s",
diff --git a/libenv/sysinfo.c b/libenv/sysinfo.c
@@ -1928,7 +1928,7 @@ static int Linux_Redhat_Version(EvalContext *ctx)
    Scientific Linux don't fall through the cracks.
    */
 
-    for (int i = 0; i < strlen(relstring); i++)
+    for (size_t i = 0; i < strlen(relstring); i++)
     {
         relstring[i] = tolower(relstring[i]);
     }
@@ -2079,7 +2079,7 @@ static int Linux_Suse_Version(EvalContext *ctx)
      * SUSE with SUSE 10.0.
      */
 
-    for (int i = 0; i < strlen(relstring); i++)
+    for (size_t i = 0; i < strlen(relstring); i++)
     {
         relstring[i] = tolower(relstring[i]);
     }
diff --git a/libntech b/libntech
@@ -1 +1 @@
-Subproject commit 4b2303d09469b2bb68dec6eeae38995b51de78aa
+Subproject commit 96423683cd327acbe365ef16adf13439d986c96f
diff --git a/libpromises/crypto.c b/libpromises/crypto.c
@@ -433,7 +433,12 @@ bool SavePublicKey(const char *user, const char *digest, const RSA *key)
     int ret;
 
     ret = snprintf(keyname, sizeof(keyname), "%s-%s", user, digest);
-    if (ret >= sizeof(keyname))
+    if (ret < 0)
+    {
+        Log(LOG_LEVEL_ERR, "snprintf failed: %s", GetErrorStr());
+        return false;
+    }
+    else if ((unsigned long) ret >= sizeof(keyname))
     {
         Log(LOG_LEVEL_ERR, "USERNAME-KEY (%s-%s) string too long!",
             user, digest);
@@ -442,7 +447,12 @@ bool SavePublicKey(const char *user, const char *digest, const RSA *key)
 
     ret = snprintf(filename, sizeof(filename), "%s/ppkeys/%s.pub",
                    GetWorkDir(), keyname);
-    if (ret >= sizeof(filename))
+    if (ret < 0)
+    {
+        Log(LOG_LEVEL_ERR, "snprintf failed: %s", GetErrorStr());
+        return false;
+    }
+    else if ((unsigned long) ret >= sizeof(filename))
     {
         Log(LOG_LEVEL_ERR, "Filename too long!");
         return false;
@@ -625,7 +635,11 @@ int EncryptString(char *out, size_t out_size, const char *in, int plainlen,
 
     cipherlen += tmplen;
 
-    if(cipherlen > max_ciphertext_size)
+    if (cipherlen < 0)
+    {
+        ProgrammingError("EncryptString: chipherlen (%d) < 0", cipherlen);
+    }
+    else if ((size_t) cipherlen > max_ciphertext_size)
     {
         ProgrammingError("EncryptString: too large ciphertext written: cipherlen (%d) > max_ciphertext_size (%zd)",
                           cipherlen, max_ciphertext_size);
@@ -711,7 +725,11 @@ int DecryptString(char *out, size_t out_size, const char *in, int cipherlen,
 
     plainlen += tmplen;
 
-    if(plainlen > max_plaintext_size)
+    if (plainlen < 0)
+    {
+        ProgrammingError("DecryptString: plainlen (%d) < 0", plainlen);
+    }
+    if ((size_t) plainlen > max_plaintext_size)
     {
         ProgrammingError("DecryptString: too large plaintext written: plainlen (%d) > max_plaintext_size (%zd)",
                           plainlen, max_plaintext_size);
@@ -729,7 +747,11 @@ void DebugBinOut(char *buffer, int len, char *comment)
     char buf[CF_BUFSIZE];
     char hexStr[3];             // one byte as hex
 
-    if (len >= (sizeof(buf) / 2))       // hex uses two chars per byte
+    if (len < 0)
+    {
+        Log(LOG_LEVEL_ERR, "Debug binary print negative len param (len = %d)", len);
+    }
+    else if ((unsigned long) len >= (sizeof(buf) / 2))       // hex uses two chars per byte
     {
         Log(LOG_LEVEL_DEBUG, "Debug binary print is too large (len = %d)", len);
         return;
diff --git a/libpromises/evalfunction.c b/libpromises/evalfunction.c
@@ -8068,7 +8068,7 @@ static char *StripPatterns(char *file_buffer, const char *pattern, const char *f
         return file_buffer;
     }
 
-    int start, end, count = 0;
+    size_t start, end, count = 0;
     const size_t original_length = strlen(file_buffer);
     while (StringMatchWithPrecompiledRegex(rx, file_buffer, &start, &end))
     {
diff --git a/libpromises/ornaments.c b/libpromises/ornaments.c
@@ -39,7 +39,13 @@
  */
 static bool StringAppendPromise(char *dst, const char *src, size_t n)
 {
-    int i, j;
+    size_t i, j;
+
+    if (n == 0)
+    {
+        return false;
+    }
+
     n--;
     for (i = 0; i < n && dst[i]; i++)
     {
diff --git a/libpromises/pipes.c b/libpromises/pipes.c
@@ -176,7 +176,14 @@ int PipeWriteData(const char *base_cmd, const char *args, const char *data)
         io.read_fd, io.write_fd, args);
 
     int res = 0;
-    if (PipeWrite(&io, data) != strlen(data))
+    int written = PipeWrite(&io, data);
+    if (written < 0)
+    {
+        Log(LOG_LEVEL_ERR, "Failed to write to pipe (fd %d): %s", 
+            io.write_fd, GetErrorStr());
+        res = -1;
+    }
+    else if ((size_t) written != strlen(data))
     {
         Log(LOG_LEVEL_VERBOSE,
             "Was not able to send whole data to application '%s'.",
@@ -217,7 +224,13 @@ int PipeReadWriteData(const char *base_cmd, const char *args, const char *reques
     Log(LOG_LEVEL_DEBUG, "Opened fds %d and %d for command '%s'.",
         io.read_fd, io.write_fd, command);
 
-    if (PipeWrite(&io, request) != strlen(request))
+    int written = PipeWrite(&io, request);
+    if (written < 0) {
+        Log(LOG_LEVEL_ERR, "Failed to write to pipe (fd %d): %s",
+            io.write_fd, GetErrorStr());
+        return -1;
+    }
+    else if ((size_t) written != strlen(request))
     {
         Log(LOG_LEVEL_VERBOSE, "Couldn't send whole data to application '%s'.",
             base_cmd);
diff --git a/libpromises/processes_select.c b/libpromises/processes_select.c
@@ -594,7 +594,7 @@ static bool SelectProcRegexMatch(const char *name1, const char *name2,
         }
         else
         {
-            int s, e;
+            size_t s, e;
             return StringMatch(regex, line[i], &s, &e);
         }
     }
diff --git a/libpromises/rlist.c b/libpromises/rlist.c
@@ -1153,8 +1153,8 @@ Rlist *RlistFromSplitRegex(const char *string, const char *regex, size_t max_ent
 
     const char *sp = string;
     size_t entry_count = 0;
-    int start = 0;
-    int end = 0;
+    size_t start = 0;
+    size_t end = 0;
     Rlist *result = NULL;
     Buffer *buffer = BufferNewWithCapacity(CF_MAXVARSIZE);
 
@@ -1213,7 +1213,7 @@ Rlist *RlistFromRegexSplitNoOverflow(const char *string, const char *regex, int
 {
     Rlist *liststart = NULL;
     char node[CF_MAXVARSIZE];
-    int start, end;
+    size_t start, end;
     int count = 0;
 
     assert(max > 0); // ensured by FnCallStringSplit() before calling us
@@ -1237,7 +1237,7 @@ Rlist *RlistFromRegexSplitNoOverflow(const char *string, const char *regex, int
         {
             len = CF_MAXVARSIZE - 1;
             Log(LOG_LEVEL_WARNING,
-                "Segment in string_split() is %d bytes and will be truncated to %zu bytes",
+                "Segment in string_split() is %zu bytes and will be truncated to %zu bytes",
                 start,
                 len);
         }
diff --git a/libpromises/syntax.c b/libpromises/syntax.c
@@ -1093,7 +1093,7 @@ static JsonElement *BundleTypesToJson(void)
     while ((bundle_type = JsonIteratorNextKey(&it)))
     {
         JsonElement *promise_types = JsonObjectGetAsArray(JsonObjectGetAsObject(bundle_types, bundle_type), "promiseTypes");
-        for (int i = 0; i < SeqLength(common_promise_types); i++)
+        for (size_t i = 0; i < SeqLength(common_promise_types); i++)
         {
             const char *common_promise_type = SeqAt(common_promise_types, i);
             JsonArrayAppendString(promise_types, common_promise_type);
diff --git a/libpromises/var_expressions.c b/libpromises/var_expressions.c
@@ -415,7 +415,7 @@ void VarRefDestroy(VarRef *ref)
         free(ref->lval);
         if (ref->num_indices > 0)
         {
-            for (int i = 0; i < ref->num_indices; ++i)
+            for (size_t i = 0; i < ref->num_indices; ++i)
             {
                 free(ref->indices[i]);
             }
diff --git a/libpromises/vars.c b/libpromises/vars.c
@@ -376,7 +376,7 @@ bool ExtractScalarReference(Buffer *out, const char *str, size_t len, bool extra
     }
 
     const char *dollar_point = memchr(str, '$', len);
-    if (!dollar_point || (dollar_point - str) == len)
+    if (!dollar_point || (size_t) (dollar_point - str) == len)
     {
         return false;
     }
diff --git a/libpromises/verify_classes.c b/libpromises/verify_classes.c
@@ -48,7 +48,8 @@ static bool ValidClassName(const char *str)
         FreeExpression(res.result);
     }
 
-    return res.result && res.position == strlen(str);
+    assert(res.position >= 0);
+    return res.result && (size_t) res.position == strlen(str);
 }
 
 PromiseResult VerifyClassPromise(EvalContext *ctx, const Promise *pp, ARG_UNUSED void *param)

Original file line number	Diff line number	Diff line change
`@@ -737,7 +737,7 @@ static bool SaveBufferCallback(const char dest_filename, void param, NewLineMo`
`737`	`737`	`if (bytes_written != BufferSize(output_buffer))`
`738`	`738`	`{`
`739`	`739`	`Log(LOG_LEVEL_ERR,`
`740`		`- "Error writing to output file '%s' when writing. %zu bytes written but expected %u. (fclose: %s)",`
	`740`	`+ "Error writing to output file '%s' when writing. %zu bytes written but expected %zu. (fclose: %s)",`
`741`	`741`	`dest_filename, bytes_written, BufferSize(output_buffer), GetErrorStr());`
`742`	`742`	`fclose(fp);`
`743`	`743`	`return false;`
Original file line number	Diff line number	Diff line change
`@@ -515,7 +515,7 @@ static char follow_symlink(const char path)`
`515`	`515`	`{`
`516`	`516`	`return NULL;`
`517`	`517`	`}`
`518`		`- if (r >= sizeof(target_buf))`
	`518`	`+ if ((size_t) r >= sizeof(target_buf))`
`519`	`519`	`{`
`520`	`520`	`Log(LOG_LEVEL_ERR, "Symlink target path too long: %s", path);`
`521`	`521`	`return NULL;`
`@@ -548,7 +548,7 @@ size_t diagnose_files(`
`548`	`548`	`*corrupt = SeqNew(length, free);`
`549`	`549`	`}`
`550`	`550`
`551`		`- for (int i = 0; i < length; ++i)`
	`551`	`+ for (size_t i = 0; i < length; ++i)`
`552`	`552`	`{`
`553`	`553`	`const char *filename = SeqAt(filenames, i);`
`554`	`554`	`const char *symlink = NULL; // Only initialized because of gcc warning`
Original file line number	Diff line number	Diff line change
`@@ -458,7 +458,7 @@ int dump_main(int argc, const char const const argv)`
`458`	`458`	`dump_mode mode = DUMP_NICE;`
`459`	`459`	`size_t offset = 1;`
`460`	`460`
`461`		`- if (argc > offset && argv[offset] != NULL && argv[offset][0] == '-')`
	`461`	`+ if ((size_t) argc > offset && argv[offset] != NULL && argv[offset][0] == '-')`
`462`	`462`	`{`
`463`	`463`	`const char *const option = argv[offset];`
`464`	`464`	`offset += 1;`
`@@ -491,7 +491,7 @@ int dump_main(int argc, const char const const argv)`
`491`	`491`	`}`
`492`	`492`	`}`
`493`	`493`
`494`		`- if (argc > offset && argv[offset] != NULL && argv[offset][0] == '-')`
	`494`	`+ if ((size_t) argc > offset && argv[offset] != NULL && argv[offset][0] == '-')`
`495`	`495`	`{`
`496`	`496`	`print_usage();`
`497`	`497`	`printf("Only one option supported!\n");`
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@`
`10`	`10`
`11`	`11`	`static void lmdump_print_hex(const char *s, size_t len)`
`12`	`12`	`{`
`13`		`- for (int i = 0; i < len; i++)`
	`13`	`+ for (size_t i = 0; i < len; i++)`
`14`	`14`	`{`
`15`	`15`	`printf("%02x", s[i]);`
`16`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -249,7 +249,7 @@ int repair_lmdb_files(Seq *files, bool force)`
`249`	`249`	`const size_t length = SeqLength(corrupt);`
`250`	`250`	`assert(length > 0);`
`251`	`251`	`backup_files_copy(corrupt);`
`252`		`- for (int i = 0; i < length; ++i)`
	`252`	`+ for (size_t i = 0; i < length; ++i)`
`253`	`253`	`{`
`254`	`254`	`const char *file = SeqAt(corrupt, i);`
`255`	`255`	`if (repair_lmdb_file(file, -1) == -1)`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,8 @@ Seq *default_lmdb_files()`
`22`	`22`	`Seq *argv_to_lmdb_files(`
`23`	`23`	`const int argc, const char const const argv, const size_t offset)`
`24`	`24`	`{`
`25`		`- if (offset >= argc)`
	`25`	`+ assert(argc >= 0);`
	`26`	`+ if (offset >= (size_t) argc)`
`26`	`27`	`{`
`27`	`28`	`Log(LOG_LEVEL_INFO,`
`28`	`29`	`"No filenames specified, defaulting to .lmdb files in %s",`