Fix warnings in public headers (#998)

Co-authored-by: Waqar Ahmed Khan <waahm7@gmail.com>

.github/workflows/ci.yml

@@ -6,7 +6,7 @@ on:
       - 'main'
 
 env:
-  BUILDER_VERSION: v0.9.40
+  BUILDER_VERSION: v0.9.43
   BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net
   BUILDER_SOURCE: releases
   PACKAGE_NAME: aws-c-common
@@ -125,7 +125,7 @@ jobs:
         python builder.pyz build -p ${{ env.PACKAGE_NAME }} --target windows-${{ matrix.arch }} --compiler msvc-16
 
   windows-vc15:
-    runs-on: windows-2019 # windows-2019 is last env with Visual Studio 2017 (v15.0)
+    runs-on: windows-2022 # latest
     strategy:
       matrix:
         arch: [x86, x64]

CMakeLists.txt

@@ -178,7 +178,7 @@ aws_set_common_properties(${PROJECT_NAME} NO_WEXTRA)
 aws_prepare_symbol_visibility_args(${PROJECT_NAME} "AWS_COMMON")
 target_compile_options(${PROJECT_NAME} PUBLIC ${PLATFORM_CFLAGS})
 
-aws_check_headers(${PROJECT_NAME} ${AWS_COMMON_HEADERS} ${AWS_TEST_HEADERS} ${AWS_COMMON_OS_HEADERS})
+aws_check_headers(${PROJECT_NAME} ${AWS_COMMON_HEADERS})
 
 #apple source already includes the definitions we want, and setting this posix source
 #version causes it to revert to an older version. So don't turn it on there, we don't need it.

cmake/AwsCheckHeaders.cmake

@@ -20,14 +20,14 @@ function(aws_check_headers target)
         set(HEADER_CHECKER_ROOT "${CMAKE_CURRENT_BINARY_DIR}/header-checker")
 
         # Write stub main file
-        set(HEADER_CHECKER_MAIN "${HEADER_CHECKER_ROOT}/stub.c")
+        set(HEADER_CHECKER_MAIN "${HEADER_CHECKER_ROOT}/headerchecker_main.c")
         file(WRITE ${HEADER_CHECKER_MAIN} "
             int main(int argc, char **argv) {
                 (void)argc;
                 (void)argv;
 
                 return 0;
-            }")
+            }\n")
 
         set(HEADER_CHECKER_LIB ${target}-header-check)
         add_executable(${HEADER_CHECKER_LIB} ${HEADER_CHECKER_MAIN})
@@ -40,20 +40,31 @@ function(aws_check_headers target)
             LINKER_LANGUAGE CXX
             CXX_STANDARD 11
             CXX_STANDARD_REQUIRED 0
-            C_STANDARD 90
+            C_STANDARD 99
         )
 
+        # Ensure our headers can be included by an application with its warnings set very high
+        if(MSVC)
+            target_compile_options(${HEADER_CHECKER_LIB} PRIVATE /Wall /WX)
+        else()
+            target_compile_options(${HEADER_CHECKER_LIB} PRIVATE -Wall -Wextra -Wpedantic -Werror)
+        endif()
+
         foreach(header IN LISTS ARGN)
             if (NOT ${header} MATCHES "\\.inl$")
-                file(RELATIVE_PATH rel_header ${CMAKE_HOME_DIRECTORY} ${header})
-                file(RELATIVE_PATH include_path "${CMAKE_HOME_DIRECTORY}/include" ${header})
-                set(stub_dir "${HEADER_CHECKER_ROOT}/${rel_header}")
-                file(MAKE_DIRECTORY "${stub_dir}")
+                # create unique token for this file, e.g.:
+                # "${CMAKE_CURRENT_SOURCE_DIR}/include/aws/common/byte_buf.h" -> "aws_common_byte_buf_h"
+                file(RELATIVE_PATH include_path "${CMAKE_CURRENT_SOURCE_DIR}/include" ${header})
+                # replace non-alphanumeric characters with underscores
+                string(REGEX REPLACE "[^a-zA-Z0-9]" "_" unique_token ${include_path})
+                set(c_file "${HEADER_CHECKER_ROOT}/headerchecker_${unique_token}.c")
+                set(cpp_file "${HEADER_CHECKER_ROOT}/headerchecker_${unique_token}.cpp")
                 # include header twice to check for include-guards
-                file(WRITE "${stub_dir}/check.c" "#include <${include_path}>\n#include <${include_path}>\n")
-                file(WRITE "${stub_dir}/checkcpp.cpp" "#include <${include_path}>\n#include <${include_path}>\n")
+                # define a unique int or compiler complains that there's nothing in the file
+                file(WRITE "${c_file}" "#include <${include_path}>\n#include <${include_path}>\nint ${unique_token}_c;\n")
+                file(WRITE "${cpp_file}" "#include <${include_path}>\n#include <${include_path}>\nint ${unique_token}_cpp;")
 
-                target_sources(${HEADER_CHECKER_LIB} PUBLIC "${stub_dir}/check.c" "${stub_dir}/checkcpp.cpp")
+                target_sources(${HEADER_CHECKER_LIB} PUBLIC "${c_file}" "${cpp_file}")
             endif()
         endforeach(header)
     endif() # PERFORM_HEADER_CHECK

include/aws/common/allocator.h

@@ -9,6 +9,7 @@
 #include <aws/common/stdbool.h>
 #include <aws/common/stdint.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
 AWS_EXTERN_C_BEGIN
 
 /* Allocator structure. An instance of this will be passed around for anything needing memory allocation */
@@ -205,5 +206,6 @@ AWS_COMMON_API
 size_t aws_small_block_allocator_page_size_available(struct aws_allocator *sba_allocator);
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_ALLOCATOR_H */

include/aws/common/array_list.h

@@ -10,6 +10,8 @@
 
 #include <stdlib.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
+
 #define AWS_ARRAY_LIST_DEBUG_FILL 0xDD
 
 struct aws_array_list {
@@ -211,13 +213,14 @@ void aws_array_list_swap(struct aws_array_list *AWS_RESTRICT list, size_t a, siz
 /**
  * Sort elements in the list in-place according to the comparator function.
  */
-AWS_STATIC_IMPL
+AWS_COMMON_API
 void aws_array_list_sort(struct aws_array_list *AWS_RESTRICT list, aws_array_list_comparator_fn *compare_fn);
 
 #ifndef AWS_NO_STATIC_IMPL
 #    include <aws/common/array_list.inl>
 #endif /* AWS_NO_STATIC_IMPL */
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_ARRAY_LIST_H */

include/aws/common/array_list.inl

@@ -96,18 +96,6 @@ bool aws_array_list_is_valid(const struct aws_array_list *AWS_RESTRICT list) {
     return required_size_is_valid && current_size_is_valid && data_is_valid && item_size_is_valid;
 }
 
-AWS_STATIC_IMPL
-void aws_array_list_debug_print(const struct aws_array_list *list) {
-    printf(
-        "arraylist %p. Alloc %p. current_size %zu. length %zu. item_size %zu. data %p\n",
-        (void *)list,
-        (void *)list->alloc,
-        list->current_size,
-        list->length,
-        list->item_size,
-        (void *)list->data);
-}
-
 AWS_STATIC_IMPL
 void aws_array_list_clean_up(struct aws_array_list *AWS_RESTRICT list) {
     AWS_PRECONDITION(AWS_IS_ZEROED(*list) || aws_array_list_is_valid(list));
@@ -404,15 +392,6 @@ int aws_array_list_set_at(struct aws_array_list *AWS_RESTRICT list, const void *
     return AWS_OP_SUCCESS;
 }
 
-AWS_STATIC_IMPL
-void aws_array_list_sort(struct aws_array_list *AWS_RESTRICT list, aws_array_list_comparator_fn *compare_fn) {
-    AWS_PRECONDITION(aws_array_list_is_valid(list));
-    if (list->data) {
-        qsort(list->data, aws_array_list_length(list), list->item_size, compare_fn);
-    }
-    AWS_POSTCONDITION(aws_array_list_is_valid(list));
-}
-
 AWS_EXTERN_C_END
 
 #endif /*  AWS_COMMON_ARRAY_LIST_INL */

include/aws/common/assert.h

@@ -10,6 +10,7 @@
 #include <aws/common/macros.h>
 #include <stdio.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
 AWS_EXTERN_C_BEGIN
 
 AWS_COMMON_API
@@ -59,15 +60,15 @@ AWS_EXTERN_C_END
 #if defined(CBMC)
 #    include <assert.h>
 #    define AWS_ASSERT(cond) assert(cond)
-#elif defined(DEBUG_BUILD) || __clang_analyzer__
+#elif defined(DEBUG_BUILD) || defined(__clang_analyzer__)
 #    define AWS_ASSERT(cond) AWS_FATAL_ASSERT(cond)
 #else
 #    define AWS_ASSERT(cond)
 #endif /*  defined(CBMC) */
 
 #if defined(CBMC)
 #    define AWS_FATAL_ASSERT(cond) AWS_ASSERT(cond)
-#elif __clang_analyzer__
+#elif defined(__clang_analyzer__)
 #    define AWS_FATAL_ASSERT(cond)                                                                                     \
         if (!(cond)) {                                                                                                 \
             abort();                                                                                                   \
@@ -187,4 +188,6 @@ AWS_EXTERN_C_END
 #define AWS_OBJECT_PTR_IS_READABLE(ptr) AWS_MEM_IS_READABLE((ptr), sizeof(*(ptr)))
 #define AWS_OBJECT_PTR_IS_WRITABLE(ptr) AWS_MEM_IS_WRITABLE((ptr), sizeof(*(ptr)))
 
+AWS_POP_SANE_WARNING_LEVEL
+
 #endif /* AWS_COMMON_ASSERT_H */

include/aws/common/atomics.h

@@ -8,6 +8,8 @@
  * SPDX-License-Identifier: Apache-2.0.
  */
 
+AWS_PUSH_SANE_WARNING_LEVEL
+
 /**
  * struct aws_atomic_var represents an atomic variable - a value which can hold an integer or pointer
  * that can be manipulated atomically. struct aws_atomic_vars should normally only be manipulated
@@ -323,5 +325,6 @@ void aws_atomic_thread_fence(enum aws_memory_order order);
 #endif /* AWS_NO_STATIC_IMPL */
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif

include/aws/common/atomics_msvc.inl

@@ -10,7 +10,11 @@
 #include <aws/common/atomics.h>
 #include <aws/common/common.h>
 
+/* This file generates level 4 compiler warnings in Visual Studio 2017 and older */
+#pragma warning(push, 3)
 #include <intrin.h>
+#pragma warning(pop)
+
 #include <stdint.h>
 #include <stdlib.h>
 
@@ -140,7 +144,7 @@ static inline void aws_atomic_priv_barrier_after(enum aws_memory_order order, en
  */
 AWS_STATIC_IMPL
 void aws_atomic_init_int(volatile struct aws_atomic_var *var, size_t n) {
-    AWS_ATOMIC_VAR_INTVAL(var) = n;
+    AWS_ATOMIC_VAR_INTVAL(var) = (aws_atomic_impl_int_t)n;
 }
 
 /**
@@ -158,7 +162,7 @@ void aws_atomic_init_ptr(volatile struct aws_atomic_var *var, void *p) {
 AWS_STATIC_IMPL
 size_t aws_atomic_load_int_explicit(volatile const struct aws_atomic_var *var, enum aws_memory_order memory_order) {
     aws_atomic_priv_barrier_before(memory_order, aws_atomic_priv_load);
-    size_t result = AWS_ATOMIC_VAR_INTVAL(var);
+    size_t result = (size_t)AWS_ATOMIC_VAR_INTVAL(var);
     aws_atomic_priv_barrier_after(memory_order, aws_atomic_priv_load);
     return result;
 }
@@ -181,10 +185,10 @@ AWS_STATIC_IMPL
 void aws_atomic_store_int_explicit(volatile struct aws_atomic_var *var, size_t n, enum aws_memory_order memory_order) {
     if (memory_order != aws_memory_order_seq_cst) {
         aws_atomic_priv_barrier_before(memory_order, aws_atomic_priv_store);
-        AWS_ATOMIC_VAR_INTVAL(var) = n;
+        AWS_ATOMIC_VAR_INTVAL(var) = (aws_atomic_impl_int_t)n;
         aws_atomic_priv_barrier_after(memory_order, aws_atomic_priv_store);
     } else {
-        AWS_INTERLOCKED_INT(Exchange)(&AWS_ATOMIC_VAR_INTVAL(var), n);
+        AWS_INTERLOCKED_INT(Exchange)(&AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)n);
     }
 }
 
@@ -213,7 +217,7 @@ size_t aws_atomic_exchange_int_explicit(
     size_t n,
     enum aws_memory_order memory_order) {
     aws_atomic_priv_check_order(memory_order);
-    return AWS_INTERLOCKED_INT(Exchange)(&AWS_ATOMIC_VAR_INTVAL(var), n);
+    return (size_t)AWS_INTERLOCKED_INT(Exchange)(&AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)n);
 }
 
 /**
@@ -244,7 +248,8 @@ bool aws_atomic_compare_exchange_int_explicit(
     aws_atomic_priv_check_order(order_success);
     aws_atomic_priv_check_order(order_failure);
 
-    size_t oldval = AWS_INTERLOCKED_INT(CompareExchange)(&AWS_ATOMIC_VAR_INTVAL(var), desired, *expected);
+    size_t oldval = (size_t)AWS_INTERLOCKED_INT(CompareExchange)(
+        &AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)desired, (aws_atomic_impl_int_t)*expected);
     bool successful = oldval == *expected;
     *expected = oldval;
 
@@ -280,7 +285,7 @@ AWS_STATIC_IMPL
 size_t aws_atomic_fetch_add_explicit(volatile struct aws_atomic_var *var, size_t n, enum aws_memory_order order) {
     aws_atomic_priv_check_order(order);
 
-    return AWS_INTERLOCKED_INT(ExchangeAdd)(&AWS_ATOMIC_VAR_INTVAL(var), n);
+    return (size_t)AWS_INTERLOCKED_INT(ExchangeAdd)(&AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)n);
 }
 
 /**
@@ -290,7 +295,7 @@ AWS_STATIC_IMPL
 size_t aws_atomic_fetch_sub_explicit(volatile struct aws_atomic_var *var, size_t n, enum aws_memory_order order) {
     aws_atomic_priv_check_order(order);
 
-    return AWS_INTERLOCKED_INT(ExchangeAdd)(&AWS_ATOMIC_VAR_INTVAL(var), -(aws_atomic_impl_int_t)n);
+    return (size_t)AWS_INTERLOCKED_INT(ExchangeAdd)(&AWS_ATOMIC_VAR_INTVAL(var), -(aws_atomic_impl_int_t)n);
 }
 
 /**
@@ -300,7 +305,7 @@ AWS_STATIC_IMPL
 size_t aws_atomic_fetch_or_explicit(volatile struct aws_atomic_var *var, size_t n, enum aws_memory_order order) {
     aws_atomic_priv_check_order(order);
 
-    return AWS_INTERLOCKED_INT(Or)(&AWS_ATOMIC_VAR_INTVAL(var), n);
+    return (size_t)AWS_INTERLOCKED_INT(Or)(&AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)n);
 }
 
 /**
@@ -310,7 +315,7 @@ AWS_STATIC_IMPL
 size_t aws_atomic_fetch_and_explicit(volatile struct aws_atomic_var *var, size_t n, enum aws_memory_order order) {
     aws_atomic_priv_check_order(order);
 
-    return AWS_INTERLOCKED_INT(And)(&AWS_ATOMIC_VAR_INTVAL(var), n);
+    return (size_t)AWS_INTERLOCKED_INT(And)(&AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)n);
 }
 
 /**
@@ -320,7 +325,7 @@ AWS_STATIC_IMPL
 size_t aws_atomic_fetch_xor_explicit(volatile struct aws_atomic_var *var, size_t n, enum aws_memory_order order) {
     aws_atomic_priv_check_order(order);
 
-    return AWS_INTERLOCKED_INT(Xor)(&AWS_ATOMIC_VAR_INTVAL(var), n);
+    return (size_t)AWS_INTERLOCKED_INT(Xor)(&AWS_ATOMIC_VAR_INTVAL(var), (aws_atomic_impl_int_t)n);
 }
 
 /**

include/aws/common/byte_buf.h

@@ -11,6 +11,8 @@
 
 #include <string.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
+
 /**
  * Represents a length-delimited binary string or buffer. If byte buffer points
  * to constant memory or memory that should otherwise not be freed by this
@@ -948,5 +950,6 @@ AWS_COMMON_API
 int aws_byte_cursor_utf8_parse_u64_hex(struct aws_byte_cursor cursor, uint64_t *dst);
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_BYTE_BUF_H */

include/aws/common/byte_order.h

@@ -8,6 +8,7 @@
 
 #include <aws/common/common.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
 AWS_EXTERN_C_BEGIN
 
 /**
@@ -70,5 +71,6 @@ AWS_STATIC_IMPL uint16_t aws_ntoh16(uint16_t x);
 #endif /* AWS_NO_STATIC_IMPL */
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_BYTE_ORDER_H */

include/aws/common/cache.h

@@ -7,6 +7,8 @@
 
 #include <aws/common/linked_hash_table.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
+
 struct aws_cache;
 
 struct aws_cache_vtable {
@@ -80,5 +82,6 @@ AWS_COMMON_API
 size_t aws_cache_get_element_count(const struct aws_cache *cache);
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_CACHE_H */

include/aws/common/clock.h

@@ -9,6 +9,8 @@
 #include <aws/common/common.h>
 #include <aws/common/math.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
+
 enum aws_timestamp_unit {
     AWS_TIMESTAMP_SECS = 1,
     AWS_TIMESTAMP_MILLIS = 1000,
@@ -57,5 +59,6 @@ int aws_sys_clock_get_ticks(uint64_t *timestamp);
 #endif /* AWS_NO_STATIC_IMPL */
 
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_CLOCK_H */

include/aws/common/command_line_parser.h

@@ -6,6 +6,8 @@
  */
 #include <aws/common/common.h>
 
+AWS_PUSH_SANE_WARNING_LEVEL
+
 enum aws_cli_options_has_arg {
     AWS_CLI_OPTIONS_NO_ARGUMENT = 0,
     AWS_CLI_OPTIONS_REQUIRED_ARGUMENT = 1,
@@ -101,5 +103,6 @@ AWS_COMMON_API int aws_cli_dispatch_on_subcommand(
     int table_length,
     void *user_data);
 AWS_EXTERN_C_END
+AWS_POP_SANE_WARNING_LEVEL
 
 #endif /* AWS_COMMON_COMMAND_LINE_PARSER_H */