Merge pull request #999 from libexpat/address-warnings

Address more clang-tidy warnings

expat/Changes

@@ -41,6 +41,7 @@ Release 2.7.2 ??? ????? ?? ????
         Other changes:
             #994  docs: Drop AppVeyor badge
            #1000  tests: Fix portable_strndup
+            #999  Address more clang-tidy warnings
 
         Special thanks to:
             Alexander Bluhm
@@ -63,7 +64,7 @@ Release 2.7.1 Thu March 27 2025
        #983 #984  Fix printf format specifiers for 32bit Emscripten
             #992  docs: Promote OpenSSF Best Practices self-certification
             #978  tests/benchmark: Resolve mistaken double close
-            #986  Address compiler warnings
+            #986  Address Frama-C warnings
        #990 #993  Version info bumped from 11:1:10 (libexpat*.so.1.10.1)
                     to 11:2:10 (libexpat*.so.1.10.2); see https://verbump.de/
                     for what these numbers do

expat/apply-clang-tidy.sh

@@ -33,6 +33,8 @@ set -e -u -o pipefail
 cd "$(dirname "$(type -P "$0")")"
 
 checks_to_enable=(
+    bugprone-narrowing-conversions
+    bugprone-suspicious-string-compare
     readability-avoid-const-params-in-decls
     readability-named-parameter
 )
@@ -75,6 +77,7 @@ else
     # https://github.com/libexpat/libexpat/issues/119
     files=( $(
         git ls-files -- \*.c | grep -v \
+        -e '^lib/xcsinc\.c$' \
         -e '^xmlwf/ct\.c$' \
         -e '^xmlwf/xmlmime\.c$' \
         -e '^xmlwf/win32filemap\.c$' \

expat/fuzz/xml_parse_fuzzer.c

@@ -15,6 +15,7 @@
  */
 
 #include <assert.h>
+#include <limits.h> // for INT_MAX
 #include <stdint.h>
 
 #include "expat.h"
@@ -65,8 +66,9 @@ ParseOneInput(XML_Parser p, const uint8_t *data, size_t size) {
   XML_SetUserData(p, p);
   XML_SetElementHandler(p, start, end);
   XML_SetCharacterDataHandler(p, may_stop_character_handler);
-  XML_Parse(p, (const XML_Char *)data, size, 0);
-  if (XML_Parse(p, (const XML_Char *)data, size, 1) == XML_STATUS_ERROR) {
+  assert(size <= INT_MAX);
+  XML_Parse(p, (const XML_Char *)data, (int)size, 0);
+  if (XML_Parse(p, (const XML_Char *)data, (int)size, 1) == XML_STATUS_ERROR) {
     XML_ErrorString(XML_GetErrorCode(p));
   }
   XML_GetCurrentLineNumber(p);

expat/fuzz/xml_parsebuffer_fuzzer.c

@@ -15,6 +15,7 @@
  */
 
 #include <assert.h>
+#include <limits.h> // for INT_MAX
 #include <stdint.h>
 #include <string.h>
 
@@ -66,16 +67,17 @@ ParseOneInput(XML_Parser p, const uint8_t *data, size_t size) {
   XML_SetUserData(p, p);
   XML_SetElementHandler(p, start, end);
   XML_SetCharacterDataHandler(p, may_stop_character_handler);
-  void *buf = XML_GetBuffer(p, size);
+  assert(size <= INT_MAX);
+  void *buf = XML_GetBuffer(p, (int)size);
   assert(buf);
   memcpy(buf, data, size);
-  XML_ParseBuffer(p, size, 0);
-  buf = XML_GetBuffer(p, size);
+  XML_ParseBuffer(p, (int)size, 0);
+  buf = XML_GetBuffer(p, (int)size);
   if (buf == NULL) {
     return;
   }
   memcpy(buf, data, size);
-  if (XML_ParseBuffer(p, size, 1) == XML_STATUS_ERROR) {
+  if (XML_ParseBuffer(p, (int)size, 1) == XML_STATUS_ERROR) {
     XML_ErrorString(XML_GetErrorCode(p));
   }
   XML_GetCurrentLineNumber(p);

expat/lib/xmlparse.c

@@ -97,7 +97,7 @@
 #include <stddef.h>
 #include <string.h> /* memset(), memcpy() */
 #include <assert.h>
-#include <limits.h> /* UINT_MAX */
+#include <limits.h> /* INT_MAX, UINT_MAX */
 #include <stdio.h>  /* fprintf */
 #include <stdlib.h> /* getenv, rand_s */
 #include <stdint.h> /* uintptr_t */
@@ -821,11 +821,14 @@ writeRandomBytes_getrandom_nonblock(void *target, size_t count) {
     void *const currentTarget = (void *)((char *)target + bytesWrittenTotal);
     const size_t bytesToWrite = count - bytesWrittenTotal;
 
+    assert(bytesToWrite <= INT_MAX);
+
     const int bytesWrittenMore =
 #    if defined(HAVE_GETRANDOM)
-        getrandom(currentTarget, bytesToWrite, getrandomFlags);
+        (int)getrandom(currentTarget, bytesToWrite, getrandomFlags);
 #    else
-        syscall(SYS_getrandom, currentTarget, bytesToWrite, getrandomFlags);
+        (int)syscall(SYS_getrandom, currentTarget, bytesToWrite,
+                     getrandomFlags);
 #    endif
 
     if (bytesWrittenMore > 0) {
@@ -2761,8 +2764,8 @@ static XML_Bool
 storeRawNames(XML_Parser parser) {
   TAG *tag = parser->m_tagStack;
   while (tag) {
-    int bufSize;
-    int nameLen = sizeof(XML_Char) * (tag->name.strLen + 1);
+    size_t bufSize;
+    size_t nameLen = sizeof(XML_Char) * (tag->name.strLen + 1);
     size_t rawNameLen;
     char *rawNameBuf = tag->buf + nameLen;
     /* Stop if already stored.  Since m_tagStack is a stack, we can stop
@@ -2779,8 +2782,8 @@ storeRawNames(XML_Parser parser) {
     /* Detect and prevent integer overflow. */
     if (rawNameLen > (size_t)INT_MAX - nameLen)
       return XML_FALSE;
-    bufSize = nameLen + (int)rawNameLen;
-    if (bufSize > tag->bufEnd - tag->buf) {
+    bufSize = nameLen + rawNameLen;
+    if (bufSize > (size_t)(tag->bufEnd - tag->buf)) {
       char *temp = (char *)REALLOC(parser, tag->buf, bufSize);
       if (temp == NULL)
         return XML_FALSE;
@@ -3677,7 +3680,7 @@ storeAtts(XML_Parser parser, const ENCODING *enc, const char *attStr,
      and clear flags that say whether attributes were specified */
   i = 0;
   if (nPrefixes) {
-    int j; /* hash table index */
+    unsigned int j; /* hash table index */
     unsigned long version = parser->m_nsAttsVersion;
 
     /* Detect and prevent invalid shift */
@@ -3772,7 +3775,7 @@ storeAtts(XML_Parser parser, const ENCODING *enc, const char *attStr,
         if (! b)
           return XML_ERROR_UNBOUND_PREFIX;
 
-        for (j = 0; j < b->uriLen; j++) {
+        for (j = 0; j < (unsigned int)b->uriLen; j++) {
           const XML_Char c = b->uri[j];
           if (! poolAppendChar(&parser->m_tempPool, c))
             return XML_ERROR_NO_MEMORY;
@@ -6277,7 +6280,7 @@ appendAttributeValue(XML_Parser parser, const ENCODING *enc, XML_Bool isCdata,
     case XML_TOK_ENTITY_REF: {
       const XML_Char *name;
       ENTITY *entity;
-      char checkEntityDecl;
+      bool checkEntityDecl;
       XML_Char ch = (XML_Char)XmlPredefinedEntityName(
           enc, ptr + enc->minBytesPerChar, next - enc->minBytesPerChar);
       if (ch) {
@@ -7863,6 +7866,11 @@ nextScaffoldPart(XML_Parser parser) {
     dtd->scaffIndex[0] = 0;
   }
 
+  // Will casting to int be safe further down?
+  if (dtd->scaffCount > INT_MAX) {
+    return -1;
+  }
+
   if (dtd->scaffCount >= dtd->scaffSize) {
     CONTENT_SCAFFOLD *temp;
     if (dtd->scaffold) {
@@ -7894,7 +7902,7 @@ nextScaffoldPart(XML_Parser parser) {
     }
     dtd->scaffold = temp;
   }
-  next = dtd->scaffCount++;
+  next = (int)dtd->scaffCount++;
   me = &dtd->scaffold[next];
   if (dtd->scaffLevel) {
     CONTENT_SCAFFOLD *parent
@@ -8093,10 +8101,10 @@ accountingGetCurrentAmplification(XML_Parser rootParser) {
         + rootParser->m_accounting.countBytesIndirect;
   const float amplificationFactor
       = rootParser->m_accounting.countBytesDirect
-            ? (countBytesOutput
+            ? ((float)countBytesOutput
                / (float)(rootParser->m_accounting.countBytesDirect))
-            : ((lenOfShortestInclude
-                + rootParser->m_accounting.countBytesIndirect)
+            : ((float)(lenOfShortestInclude
+                       + rootParser->m_accounting.countBytesIndirect)
                / (float)lenOfShortestInclude);
   assert(! rootParser->m_parentParser);
   return amplificationFactor;

expat/tests/alloc_tests.c

@@ -323,7 +323,7 @@ START_TEST(test_alloc_run_external_parser) {
     XML_SetParamEntityParsing(g_parser, XML_PARAM_ENTITY_PARSING_ALWAYS);
     XML_SetUserData(g_parser, foo_text);
     XML_SetExternalEntityRefHandler(g_parser, external_entity_null_loader);
-    g_allocation_count = i;
+    g_allocation_count = (int)i;
     if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE)
         != XML_STATUS_ERROR)
       break;
@@ -434,7 +434,7 @@ START_TEST(test_alloc_internal_entity) {
   const unsigned int max_alloc_count = 20;
 
   for (i = 0; i < max_alloc_count; i++) {
-    g_allocation_count = i;
+    g_allocation_count = (int)i;
     XML_SetUnknownEncodingHandler(g_parser, unknown_released_encoding_handler,
                                   NULL);
     if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE)

expat/tests/basic_tests.c

@@ -412,13 +412,13 @@ START_TEST(test_utf16_le_epilog_newline) {
 
   if (first_chunk_bytes >= sizeof(text) - 1)
     fail("bad value of first_chunk_bytes");
-  if (_XML_Parse_SINGLE_BYTES(g_parser, text, first_chunk_bytes, XML_FALSE)
+  if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)first_chunk_bytes, XML_FALSE)
       == XML_STATUS_ERROR)
     xml_failure(g_parser);
   else {
     enum XML_Status rc;
     rc = _XML_Parse_SINGLE_BYTES(g_parser, text + first_chunk_bytes,
-                                 sizeof(text) - first_chunk_bytes - 1,
+                                 (int)(sizeof(text) - first_chunk_bytes - 1),
                                  XML_TRUE);
     if (rc == XML_STATUS_ERROR)
       xml_failure(g_parser);

expat/tests/handlers.c

@@ -89,15 +89,15 @@ start_element_event_handler2(void *userData, const XML_Char *name,
                              const XML_Char **attr) {
   StructData *storage = (StructData *)userData;
   UNUSED_P(attr);
-  StructData_AddItem(storage, name, XML_GetCurrentColumnNumber(g_parser),
-                     XML_GetCurrentLineNumber(g_parser), STRUCT_START_TAG);
+  StructData_AddItem(storage, name, (int)XML_GetCurrentColumnNumber(g_parser),
+                     (int)XML_GetCurrentLineNumber(g_parser), STRUCT_START_TAG);
 }
 
 void XMLCALL
 end_element_event_handler2(void *userData, const XML_Char *name) {
   StructData *storage = (StructData *)userData;
-  StructData_AddItem(storage, name, XML_GetCurrentColumnNumber(g_parser),
-                     XML_GetCurrentLineNumber(g_parser), STRUCT_END_TAG);
+  StructData_AddItem(storage, name, (int)XML_GetCurrentColumnNumber(g_parser),
+                     (int)XML_GetCurrentLineNumber(g_parser), STRUCT_END_TAG);
 }
 
 void XMLCALL
@@ -132,7 +132,7 @@ counting_start_element_handler(void *userData, const XML_Char *name,
     fail("ID not present");
     return;
   }
-  if (id != -1 && xcstrcmp(atts[id], info->id_name)) {
+  if (id != -1 && xcstrcmp(atts[id], info->id_name) != 0) {
     fail("ID does not have the correct name");
     return;
   }
@@ -147,7 +147,7 @@ counting_start_element_handler(void *userData, const XML_Char *name,
       fail("Attribute not recognised");
       return;
     }
-    if (xcstrcmp(atts[1], attr->value)) {
+    if (xcstrcmp(atts[1], attr->value) != 0) {
       fail("Attribute has wrong value");
       return;
     }
@@ -1110,7 +1110,7 @@ external_entity_devaluer(XML_Parser parser, const XML_Char *context,
   UNUSED_P(publicId);
   if (systemId == NULL || ! xcstrcmp(systemId, XCS("bar")))
     return XML_STATUS_OK;
-  if (xcstrcmp(systemId, XCS("foo")))
+  if (xcstrcmp(systemId, XCS("foo")) != 0)
     fail("Unexpected system ID");
   ext_parser = XML_ExternalEntityParserCreate(parser, context, NULL);
   if (ext_parser == NULL)
@@ -1276,7 +1276,7 @@ external_entity_duff_loader(XML_Parser parser, const XML_Char *context,
   UNUSED_P(publicId);
   /* Try a few different allocation levels */
   for (i = 0; i < max_alloc_count; i++) {
-    g_allocation_count = i;
+    g_allocation_count = (int)i;
     new_parser = XML_ExternalEntityParserCreate(parser, context, NULL);
     if (new_parser != NULL) {
       XML_ParserFree(new_parser);
@@ -1552,15 +1552,16 @@ verify_attlist_decl_handler(void *userData, const XML_Char *element_name,
                             const XML_Char *default_value, int is_required) {
   AttTest *at = (AttTest *)userData;
 
-  if (xcstrcmp(element_name, at->element_name))
+  if (xcstrcmp(element_name, at->element_name) != 0)
     fail("Unexpected element name in attribute declaration");
-  if (xcstrcmp(attr_name, at->attr_name))
+  if (xcstrcmp(attr_name, at->attr_name) != 0)
     fail("Unexpected attribute name in attribute declaration");
-  if (xcstrcmp(attr_type, at->attr_type))
+  if (xcstrcmp(attr_type, at->attr_type) != 0)
     fail("Unexpected attribute type in attribute declaration");
   if ((default_value == NULL && at->default_value != NULL)
       || (default_value != NULL && at->default_value == NULL)
-      || (default_value != NULL && xcstrcmp(default_value, at->default_value)))
+      || (default_value != NULL
+          && xcstrcmp(default_value, at->default_value) != 0))
     fail("Unexpected default value in attribute declaration");
   if (is_required != at->is_required)
     fail("Requirement mismatch in attribute declaration");
@@ -1751,7 +1752,7 @@ param_entity_match_handler(void *userData, const XML_Char *entityName,
      * going to overflow an int.
      */
     if (value_length != (int)xcstrlen(entity_value_to_match)
-        || xcstrncmp(value, entity_value_to_match, value_length)) {
+        || xcstrncmp(value, entity_value_to_match, value_length) != 0) {
       entity_match_flag = ENTITY_MATCH_FAIL;
     } else {
       entity_match_flag = ENTITY_MATCH_SUCCESS;

expat/tests/misc_tests.c

@@ -70,7 +70,7 @@ START_TEST(test_misc_alloc_create_parser) {
 
   /* Something this simple shouldn't need more than 10 allocations */
   for (i = 0; i < max_alloc_count; i++) {
-    g_allocation_count = i;
+    g_allocation_count = (int)i;
     g_parser = XML_ParserCreate_MM(NULL, &memsuite, NULL);
     if (g_parser != NULL)
       break;
@@ -90,7 +90,7 @@ START_TEST(test_misc_alloc_create_parser_with_encoding) {
 
   /* Try several levels of allocation */
   for (i = 0; i < max_alloc_count; i++) {
-    g_allocation_count = i;
+    g_allocation_count = (int)i;
     g_parser = XML_ParserCreate_MM(XCS("us-ascii"), &memsuite, NULL);
     if (g_parser != NULL)
       break;
@@ -211,7 +211,8 @@ START_TEST(test_misc_version) {
   if (! versions_equal(&read_version, &parsed_version))
     fail("Version mismatch");
 
-  if (xcstrcmp(version_text, XCS("expat_2.7.1"))) /* needs bump on releases */
+  if (xcstrcmp(version_text, XCS("expat_2.7.1"))
+      != 0) /* needs bump on releases */
     fail("XML_*_VERSION in expat.h out of sync?\n");
 }
 END_TEST

expat/tests/nsalloc_tests.c

@@ -83,7 +83,7 @@ START_TEST(test_nsalloc_xmlns) {
   const unsigned int max_alloc_count = 30;
 
   for (i = 0; i < max_alloc_count; i++) {
-    g_allocation_count = i;
+    g_allocation_count = (int)i;
     /* Exercise more code paths with a default handler */
     XML_SetDefaultHandler(g_parser, dummy_default_handler);
     if (_XML_Parse_SINGLE_BYTES(g_parser, text, (int)strlen(text), XML_TRUE)
@@ -523,7 +523,7 @@ START_TEST(test_nsalloc_realloc_binding_uri) {
   /* Now repeat with a longer URI and a duff reallocator */
   for (i = 0; i < max_realloc_count; i++) {
     XML_ParserReset(g_parser, NULL);
-    g_reallocation_count = i;
+    g_reallocation_count = (int)i;
     if (_XML_Parse_SINGLE_BYTES(g_parser, second, (int)strlen(second), XML_TRUE)
         != XML_STATUS_ERROR)
       break;

expat/xmlwf/unixfilemap.c

@@ -41,6 +41,7 @@
 #include <errno.h>
 #include <string.h>
 #include <stdio.h>
+#include <stdlib.h> // NULL
 #include <unistd.h>
 
 #ifndef MAP_FILE
@@ -93,8 +94,7 @@ filemap(const tchar *name,
     close(fd);
     return 1;
   }
-  p = (void *)mmap((void *)0, (size_t)nbytes, PROT_READ, MAP_FILE | MAP_PRIVATE,
-                   fd, (off_t)0);
+  p = mmap(NULL, nbytes, PROT_READ, MAP_FILE | MAP_PRIVATE, fd, (off_t)0);
   if (p == (void *)-1) {
     tperror(name);
     close(fd);

expat/xmlwf/xmlfile.c

@@ -56,12 +56,19 @@
 #include "xmltchar.h"
 #include "filemap.h"
 
+/* Function "read": */
 #if defined(_MSC_VER)
 #  include <io.h>
-#endif
-
-#ifdef HAVE_UNISTD_H
+/* https://msdn.microsoft.com/en-us/library/wyssk1bs(v=vs.100).aspx */
+#  define EXPAT_read _read
+#  define EXPAT_read_count_t int
+#  define EXPAT_read_req_t unsigned int
+#else /* POSIX */
 #  include <unistd.h>
+/* https://pubs.opengroup.org/onlinepubs/009695399/functions/read.html */
+#  define EXPAT_read read
+#  define EXPAT_read_count_t ssize_t
+#  define EXPAT_read_req_t size_t
 #endif
 
 #ifndef O_BINARY
@@ -192,7 +199,7 @@ processStream(const XML_Char *filename, XML_Parser parser) {
     }
   }
   for (;;) {
-    int nread;
+    EXPAT_read_count_t nread;
     char *buf = (char *)XML_GetBuffer(parser, g_read_size_bytes);
     if (! buf) {
       if (filename != NULL)
@@ -201,14 +208,14 @@ processStream(const XML_Char *filename, XML_Parser parser) {
                filename != NULL ? filename : T("xmlwf"));
       return 0;
     }
-    nread = read(fd, buf, g_read_size_bytes);
+    nread = EXPAT_read(fd, buf, (EXPAT_read_req_t)g_read_size_bytes);
     if (nread < 0) {
       tperror(filename != NULL ? filename : T("STDIN"));
       if (filename != NULL)
         close(fd);
       return 0;
     }
-    if (XML_ParseBuffer(parser, nread, nread == 0) == XML_STATUS_ERROR) {
+    if (XML_ParseBuffer(parser, (int)nread, nread == 0) == XML_STATUS_ERROR) {
       reportError(parser, filename != NULL ? filename : T("STDIN"));
       if (filename != NULL)
         close(fd);

expat/xmlwf/xmlwf.c

@@ -305,7 +305,7 @@ static XML_Char *
 xcsdup(const XML_Char *s) {
   XML_Char *result;
   int count = 0;
-  int numBytes;
+  size_t numBytes;
 
   /* Get the length of the string, including terminator */
   while (s[count++] != 0) {