CDRIVER-5745 Enable and address aggressive -Wformat warnings (#1757)

* Format mongoc-stat.c * Use <PROJECT_NAME>_SOURCE_DIR * Move source file lists into respective scopes * cmake: remove redundant include of MongoC-Warnings * cmake: refactor MongoC-Warnings to set options as a variable * cmake: reduce scope of mongoc warnings to our source code * CDRIVER-5745 promote -Wformat warnings to unconditional hard error * Address -Wformat warnings * Address portability issues with the integral type of __LINE__ * Address -Wconversion warnings * Address PRIu8 compatibility issues with GCC in release builds
mongodb · Oct 10, 2024 · aca2e1b · aca2e1b
1 parent f5de26d
commit aca2e1b
Show file tree

Hide file tree

Showing 50 changed files with 527 additions and 450 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -308,9 +308,6 @@ if(ENABLE_MAINTAINER_FLAGS)
       gnu-like:-Wall
       gnu-like:-Wempty-body
       gnu:not-gcc-lt7:-Wexpansion-to-defined
-      gnu-like:-Wformat
-      gnu-like:-Wformat-nonliteral
-      gnu-like:-Wformat-security
       gnu-like:-Winit-self
       gnu-like:-Wmissing-include-dirs
       gnu-like:-Wredundant-decls
@@ -458,31 +455,7 @@ if (ENABLE_MONGOC)
       message (FATAL_ERROR "ENABLE_TESTS requires ENABLE_STATIC or ENABLE_STATIC_BUILD")
    endif ()
 
-   set (SOURCE_DIR "${PROJECT_SOURCE_DIR}/")
-
-   set (UTF8PROC_SOURCES
-      ${SOURCE_DIR}/src/utf8proc-2.8.0/utf8proc.c
-   )
-
-   set (ZLIB_SOURCES
-      ${SOURCE_DIR}/src/zlib-1.3.1/adler32.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/crc32.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/deflate.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/infback.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/inffast.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/inflate.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/inftrees.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/trees.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/zutil.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/compress.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/uncompr.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/gzclose.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/gzlib.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/gzread.c
-      ${SOURCE_DIR}/src/zlib-1.3.1/gzwrite.c
-   )
-
-   set (CPACK_RESOURCE_FILE_LICENSE "${SOURCE_DIR}/COPYING")
+   set (CPACK_RESOURCE_FILE_LICENSE "${mongo-c-driver_SOURCE_DIR}/COPYING")
 
    include (CPack)
 
@@ -494,28 +467,6 @@ if (ENABLE_MONGOC)
       add_definitions (-Wno-deprecated-declarations)
    endif ()
 
-   set (KMS_MSG_SOURCES
-      ${SOURCE_DIR}/src/kms-message/src/hexlify.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_b64.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_caller_identity_request.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_crypto_apple.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_crypto_libcrypto.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_crypto_none.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_crypto_windows.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_decrypt_request.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_encrypt_request.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_kmip_response_parser.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_kv_list.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_message.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_port.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_request.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_request_opt.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_request_str.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_response.c
-      ${SOURCE_DIR}/src/kms-message/src/kms_response_parser.c
-      ${SOURCE_DIR}/src/kms-message/src/sort.c
-   )
-
    if (NOT ENABLE_MONGODB_AWS_AUTH MATCHES "ON|OFF|AUTO")
       message (FATAL_ERROR "ENABLE_MONGODB_AWS_AUTH option must be ON, AUTO, or OFF")
    endif ()

diff --git a/build/cmake/FindUtf8Proc.cmake b/build/cmake/FindUtf8Proc.cmake
@@ -1,6 +1,6 @@
 if(USE_BUNDLED_UTF8PROC)
   message (STATUS "Enabling utf8proc (bundled)")
-  add_library (utf8proc_obj OBJECT "${UTF8PROC_SOURCES}")
+  add_library (utf8proc_obj OBJECT "${mongo-c-driver_SOURCE_DIR}/src/utf8proc-2.8.0/utf8proc.c")
   set_property (TARGET utf8proc_obj PROPERTY POSITION_INDEPENDENT_CODE TRUE)
   target_compile_definitions (utf8proc_obj PUBLIC UTF8PROC_STATIC)
 else ()

diff --git a/build/cmake/MongoC-Warnings.cmake b/build/cmake/MongoC-Warnings.cmake
@@ -1,17 +1,17 @@
 #[[
-   This file sets warning options for the directories in which it is include()'d
+   This file sets warning options in the "mongoc-warning-options" variable when included.
 
    These warnings are intended to be ported to each supported platform, and
    especially for high-value warnings that are very likely to catch latent bugs
    early in the process before the code is even run.
 ]]
 
 #[[
-   Define additional compile options, conditional on the compiler being used.
+   Appends additional compile options to the "mongoc-warning-options" variable, conditioned
+   on the compiler being used.
+
    Each option should be prefixed by `gnu:`, `clang:`, `msvc:`, or `gnu-like:`.
    Those options will be conditionally enabled for GCC, Clang, or MSVC.
-
-   These options are attached to the source directory and its children.
 ]]
 function (mongoc_add_warning_options)
    list(APPEND CMAKE_MESSAGE_CONTEXT ${CMAKE_CURRENT_FUNCTION})
@@ -26,6 +26,10 @@ function (mongoc_add_warning_options)
    # "Old" GNU is GCC < 5, which is missing several warning options
    set(cond/gcc-lt5 $<AND:${cond/gnu},$<VERSION_LESS:$<C_COMPILER_VERSION>,5>>)
    set(cond/gcc-lt7 $<AND:${cond/gnu},$<VERSION_LESS:$<C_COMPILER_VERSION>,7>>)
+   set(cond/gcc-lt8 $<AND:${cond/gnu},$<VERSION_LESS:$<C_COMPILER_VERSION>,8>>)
+   set(cond/gcc-lt11 $<AND:${cond/gnu},$<VERSION_LESS:$<C_COMPILER_VERSION>,11>>)
+   set(cond/clang-lt10 $<AND:${cond/clang},$<VERSION_LESS:$<C_COMPILER_VERSION>,10>>)
+   set(cond/clang-lt19 $<AND:${cond/clang},$<VERSION_LESS:$<C_COMPILER_VERSION>,19>>)
    # Process options:
    foreach (opt IN LISTS ARGV)
       # Replace prefixes. Matches right-most first:
@@ -53,12 +57,15 @@ function (mongoc_add_warning_options)
          set(opt "${before}${opt}")
          message(TRACE "Become: ${opt}")
       endwhile ()
-      add_compile_options("${opt}")
+      list(APPEND mongoc-warning-options "${opt}")
    endforeach ()
+   set(mongoc-warning-options "${mongoc-warning-options}" PARENT_SCOPE)
 endfunction ()
 
 set (is_c_lang "$<COMPILE_LANGUAGE:C>")
 
+set (mongoc-warning-options "")
+
 # These below warnings should always be unconditional hard errors, as the code is
 # almost definitely broken
 mongoc_add_warning_options (
@@ -77,6 +84,22 @@ mongoc_add_warning_options (
      # Definite use of uninitialized value
      gnu-like:-Werror=uninitialized msvc:/we4700
 
+     # Format strings.
+     gnu-like:-Werror=format
+     gnu-like:-Werror=format=2
+     # GCC does not document the full list of warnings enabled by -Wformat.
+     # For assurance, explicitly include those not listed by -Wformat or -Wformat=2.
+     gnu:not-gcc-lt11:-Werror=format-diag
+     gnu:not-gcc-lt8:-Werror=format-overflow=2
+     gnu:not-gcc-lt5:-Werror=format-signedness
+     gnu:not-gcc-lt8:-Werror=format-truncation=2
+     # Clang does not include several flags in `-Wformat` or `-Wformat=2`.
+     # For assurance, explicitly include those not listed by -Wformat or -Wformat=2.
+     clang:-Werror=format-non-iso
+     clang:-Werror=format-pedantic
+     clang:not-clang-lt19:-Werror=format-signedness
+     clang:not-clang-lt10:-Werror=format-type-confusion
+
      # Aside: Disable CRT insecurity warnings
      msvc:/D_CRT_SECURE_NO_WARNINGS
      )
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -4,7 +4,7 @@
 
 # Copy zconf.h.in to zconf.h, used by zlib
 configure_file (
-   "${SOURCE_DIR}/src/zlib-1.3.1/zconf.h.in"
+   "${mongo-c-driver_SOURCE_DIR}/src/zlib-1.3.1/zconf.h.in"
    "${CMAKE_BINARY_DIR}/src/zlib-1.3.1/zconf.h"
    COPYONLY
 )

diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt
@@ -5,8 +5,6 @@ if (ENABLE_DEBUG_ASSERTIONS)
     set (MONGOC_ENABLE_DEBUG_ASSERTIONS 1)
 endif ()
 
-include (MongoC-Warnings)
-
 configure_file (
         "${PROJECT_SOURCE_DIR}/src/common/common-config.h.in"
         "${PROJECT_BINARY_DIR}/src/common/common-config.h"

diff --git a/src/libbson/CMakeLists.txt b/src/libbson/CMakeLists.txt
@@ -223,6 +223,9 @@ mongo_target_requirements(
          MCOMMON_NAME_PREFIX=_bson_mcommon
    COMPILE_OPTIONS
       PRIVATE
+         # Enable unconditional warnings-as-errors for our source code.
+         ${mongoc-warning-options}
+
          # Macro constant INFINITY triggers constant arithmetic overflow warnings in
          # VS 2013, but VS 2013 doesn't support inline warning suppression.
          # Remove once support for VS 2013 is dropped.
@@ -267,6 +270,9 @@ if (ENABLE_EXAMPLES)
       add_executable (${bin} EXCLUDE_FROM_ALL ${src})
       add_dependencies (mongo_c_driver_examples ${bin})
 
+      # Enable unconditional warnings-as-errors for our source code.
+      target_compile_options (${bin} PRIVATE ${mongoc-warning-options})
+
       # Link against the shared lib like normal apps
       if(TARGET bson_shared)
          target_link_libraries (${bin} bson_shared)

diff --git a/src/libbson/examples/bson-validate.c b/src/libbson/examples/bson-validate.c
@@ -73,7 +73,7 @@ main (int argc, char *argv[])
       while ((b = bson_reader_read (reader, NULL))) {
          docnum++;
          if (!bson_validate (b, (BSON_VALIDATE_UTF8 | BSON_VALIDATE_UTF8_ALLOW_NULL), &offset)) {
-            fprintf (stderr, "Document %u in \"%s\" is invalid at offset %u.\n", docnum, filename, (int) offset);
+            fprintf (stderr, "Document %d in \"%s\" is invalid at offset %zu.\n", docnum, filename, offset);
             bson_reader_destroy (reader);
             return 1;
          }

diff --git a/src/libbson/src/bson/bson-macros.h b/src/libbson/src/bson/bson-macros.h
@@ -190,24 +190,25 @@
 #define BSON_FUNC __func__
 #endif
 
-#define BSON_ASSERT(test)                                                                                 \
-   do {                                                                                                   \
-      if (!(BSON_LIKELY (test))) {                                                                        \
-         fprintf (stderr, "%s:%d %s(): precondition failed: %s\n", __FILE__, __LINE__, BSON_FUNC, #test); \
-         abort ();                                                                                        \
-      }                                                                                                   \
+#define BSON_ASSERT(test)                                                                                         \
+   do {                                                                                                           \
+      if (!(BSON_LIKELY (test))) {                                                                                \
+         fprintf (stderr, "%s:%d %s(): precondition failed: %s\n", __FILE__, (int) (__LINE__), BSON_FUNC, #test); \
+         abort ();                                                                                                \
+      }                                                                                                           \
    } while (0)
 
 /**
  * @brief Assert the expression `Assertion`, and evaluates to `Value` on
  * success.
  */
-#define BSON_ASSERT_INLINE(Assertion, Value)                                                                         \
-   ((void) ((Assertion)                                                                                              \
-               ? (0)                                                                                                 \
-               : ((fprintf (stderr, "%s:%d %s(): Assertion '%s' failed", __FILE__, __LINE__, BSON_FUNC, #Assertion), \
-                   abort ()),                                                                                        \
-                  0)),                                                                                               \
+#define BSON_ASSERT_INLINE(Assertion, Value)                                                                           \
+   ((void) ((Assertion)                                                                                                \
+               ? (0)                                                                                                   \
+               : ((fprintf (                                                                                           \
+                      stderr, "%s:%d %s(): Assertion '%s' failed", __FILE__, (int) (__LINE__), BSON_FUNC, #Assertion), \
+                   abort ()),                                                                                          \
+                  0)),                                                                                                 \
     Value)
 
 /**
@@ -349,10 +350,10 @@
  * @param What A string to include in the error message if this point is ever
  * executed.
  */
-#define BSON_UNREACHABLE(What)                                                                             \
-   do {                                                                                                    \
-      fprintf (stderr, "%s:%d %s(): Unreachable code reached: %s\n", __FILE__, __LINE__, BSON_FUNC, What); \
-      abort ();                                                                                            \
+#define BSON_UNREACHABLE(What)                                                                                     \
+   do {                                                                                                            \
+      fprintf (stderr, "%s:%d %s(): Unreachable code reached: %s\n", __FILE__, (int) (__LINE__), BSON_FUNC, What); \
+      abort ();                                                                                                    \
    } while (0)
 
 /**

diff --git a/src/libbson/tests/test-bson-corpus.c b/src/libbson/tests/test-bson-corpus.c
@@ -4,6 +4,8 @@
 #include "corpus-test.h"
 #include <mcd-string.h>
 
+#include <inttypes.h>
+
 
 #define IS_NAN(dec) (dec).high == 0x7c00000000000000ull
 
@@ -47,15 +49,16 @@ compare_data (const uint8_t *a, uint32_t a_len, const uint8_t *b, uint32_t b_len
    if (a_len != b_len || memcmp (a, b, (size_t) a_len)) {
       a_str = mcd_string_new (NULL);
       for (i = 0; i < a_len; i++) {
-         mcd_string_append_printf (a_str, "%02X", (int) a[i]);
+         mcd_string_append_printf (a_str, "%02" PRIx8, a[i]);
       }
 
       b_str = mcd_string_new (NULL);
       for (i = 0; i < b_len; i++) {
-         mcd_string_append_printf (b_str, "%02X", (int) b[i]);
+         mcd_string_append_printf (b_str, "%02" PRIx8, b[i]);
       }
 
-      fprintf (stderr, "unequal data of length %d and %d:\n%s\n%s\n", a_len, b_len, a_str->str, b_str->str);
+      fprintf (
+         stderr, "unequal data of length %" PRIu32 " and %" PRIu32 ":\n%s\n%s\n", a_len, b_len, a_str->str, b_str->str);
 
       abort ();
    }

diff --git a/src/libbson/tests/test-bson-error.c b/src/libbson/tests/test-bson-error.c
@@ -24,10 +24,10 @@ test_bson_error_basic (void)
 {
    bson_error_t error;
 
-   bson_set_error (&error, 123, 456, "%s %u", "localhost", 27017);
+   bson_set_error (&error, 123, 456, "%s %d", "localhost", 27017);
    BSON_ASSERT (!strcmp (error.message, "localhost 27017"));
-   ASSERT_CMPINT (error.domain, ==, 123);
-   ASSERT_CMPINT (error.code, ==, 456);
+   ASSERT_CMPUINT32 (error.domain, ==, 123u);
+   ASSERT_CMPUINT32 (error.code, ==, 456u);
 }
 
 

diff --git a/src/libbson/tests/test-bson.c b/src/libbson/tests/test-bson.c
@@ -1129,7 +1129,7 @@ test_bson_validate (void)
    bson_error_t error;
 
    for (i = 1; i <= 38; i++) {
-      bson_snprintf (filename, sizeof filename, "test%u.bson", i);
+      bson_snprintf (filename, sizeof filename, "test%d.bson", i);
       b = get_bson (filename);
       BSON_ASSERT (bson_validate (b, BSON_VALIDATE_NONE, &offset));
       bson_destroy (b);
@@ -2218,10 +2218,9 @@ test_bson_iter_key_len (void)
    BSON_ASSERT (bson_iter_init (&iter, bson));
    while (bson_iter_next (&iter)) {
       ASSERT_WITH_MSG (strlen (bson_iter_key (&iter)) == bson_iter_key_len (&iter),
-                       "iter_key_len differs from real key length. got %d but "
-                       "expected %d for key %s\n",
+                       "iter_key_len differs from real key length. got %" PRIu32 " but expected %zu for key %s\n",
                        bson_iter_key_len (&iter),
-                       (int) strlen (bson_iter_key (&iter)),
+                       strlen (bson_iter_key (&iter)),
                        bson_iter_key (&iter));
    }
 }
@@ -2250,8 +2249,8 @@ test_bson_iter_init_from_data_at_offset (void)
          for (i = 0; i < 2; i++) {
             fprintf (stderr, "iter %d: ", i);
             fprintf (stderr,
-                     "len=%d, off=%d, type=%d, key=%d, d1=%d, d2=%d, "
-                     "d3=%d, d4=%d, next_off=%d, err_off=%d\n",
+                     "len=%" PRIu32 ", off=%" PRIu32 ", type=%" PRIu32 ", key=%" PRIu32 ", d1=%" PRIu32 ", d2=%" PRIu32
+                     ", d3=%" PRIu32 ", d4=%" PRIu32 ", next_off=%" PRIu32 ", err_off=%" PRIu32 "\n",
                      iters[i]->len,
                      iters[i]->off,
                      iters[i]->type,

diff --git a/src/libbson/tests/test-string.c b/src/libbson/tests/test-string.c
@@ -131,7 +131,7 @@ test_bson_strdup_printf (void)
 {
    char *s;
 
-   s = bson_strdup_printf ("%s:%u", "localhost", 27017);
+   s = bson_strdup_printf ("%s:%d", "localhost", 27017);
    BSON_ASSERT (!strcmp (s, "localhost:27017"));
    bson_free (s);
 }