Skip to content

CDRIVER-4599 Reducing Warnings - MSVC and MinGW Compilation Warnings #1229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 102 commits into from
Apr 14, 2023

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Apr 3, 2023

Description

This PR is a followup to #1221 that resolves CDRIVER-4599 by addressing all remaining compilation warnings generated by MSVC and MinGW on the current Evergreen matrix for the following CMake targets:

  • bson_shared
  • bson_static
  • add_example executables under src/libbson
  • mongoc_shared
  • mongoc_static
  • test-libmongoc-lib
  • mongoc_add_test executables under src/libmongoc
  • mongoc_add_example executables under src/libmongoc

Verified by this patch, which temporarily adds /WX or -Werror to the aforementioned CMake targets under src/libbson and src/libmongoc. As with the prior PR, these flags are not included in this PR.

The scope of this PR is limited to MSVC and MinGW warnings for the aforementioned CMake targets. A followup PR may eventually address warnings generated by GCC and Clang as well.

Stats

Given the same method as described in #1221, relative to 9ef1cc9, this PR reduces the total number of warnings emitted by:

  • GCC: 4379 -> 3809 (-13.0%)
  • Clang: 4338 -> 3817 (-12.0%)
  • MSVC: 235 -> 0 (-100%) !

and the total number of unique warnings by:

  • GCC: 1315 -> 967 (-26.5%)
  • Clang: 1427 -> 1113 (-22.0%)
  • MSVC: 150 -> 0 (-100%) !

Note: the warning counts do not include D9025 command line option override warnings generated by the kms_message and zlib libraries.

Methodology

In addition to the methods described in #1221:

-Wmissing-braces Warnings

The MCD_AZURE_IMDS_REQUEST_INIT macro was added to mcd-azure.h to address this warning when initializing mcd_azure_imds_request objects.

The _init data member was added to the union mongoc_rpc_t in mongoc-rpc-private.h to address this warning. The _init data member is only used to ensure the object is zero-initialized without unnecessarily specifying a specific union member as being active by default.

C4091 Warning Generated by DbgHelp.h

This warning was observed when compiling on some MSVC versions. Opted to suppress this warning as it is not caused by our code (source of warning is in DbgHelp.h).

C4756 Constant Arithmetic Overflow Warnings

The use of the INFINITY macro in test-json.c prompted the addition of the /wd4756 compile option for the test-libmongoc-lib library as done in #1221.

C4996 Deprecated Function Warnings

This is already tracked by CDRIVER-4263. Suppressions were added accordingly.

Changes to Internal APIs

Some notable changes to private interfaces include:

mongoc-set-private.h

Changed the idx parameter type from int to size_t for functions mongoc_set_get_item*.

mongoc-gridfs-bucket-file-private.h

Changed the bytes_read data member type in struct mongoc_gridfs_bucket_file_t from int32_t to size_t.

mongoc-buffer-private.h

Changed the timeout_msec parameter type from int32_t to int64_t for functions _mongoc_buffer_*, which now either return an error or log an error message when timeout_msec is not representable as an int32_t.

The scope of applicable changes was limited by CDRIVER-4589.

mongoc-stream-private.h

Changed the timeout_msec parameter type from int32_t to int64_t for function _mongoc_stream_writev_full, which now returns an error if timeout_msec is not representable as an int32_t.

The scope of applicable changes was limited by CDRIVER-4589.

mongoc-server-monitor.c

Changed the (min_)heartbeat_frequency_ms data member types in struct _mongoc_server_monitor_t from uint64_t to int64_t.

Changed the request_id data member type in struct _mongoc_server_monitor_t from int64_t to int32_t.

The function _server_monitor_awaitable_hello now appends maxAwaitTimeMS to the awaitable hello command document as an Int64 instead of an Int32!

Scope of applicable changes was limited by CDRIVER-4589.

mongoc-stream-tls-private.h

Changed the timeout_msec data member type from int32_t to int64_t, which now causes the following functions to log an error message if timeout_msec is not representable as an int32_t:

  • mongoc_secure_channel_(read|write)
  • mongoc_stream_tls_openssl_bio_(read|write)

The scope of applicable changes was limited by CDRIVER-4589.

mongoc-secure-channel-private.h

Changed the return type of functions mongoc_secure_channel_(read|write) from size_t to ssize_t.

Moved declaration ownership of the function mongoc_secure_channel_write into mongoc-secure-channel-private.h (previously manually declared in mongoc-stream-tls-secure-channel.c).

mongoc-topology-private.h

Changed the max_hosts parameter type for function _mongoc_apply_srv_max_hosts from int32_t to size_t.

mongoc-topology-description-private.h

Changed the local_threshold_ms parameter type for function mongoc_topology_description_suitable_servers from size_t to int64_t.

mongoc-host-list-private.h

Changed the return type of function _mongoc_host_list_length from int to size_t.

mongoc-cluster-aws-private.h

Changed the sts_fqdn_len parameter type from uint32_t to size_t for _mongoc_validate_and_derive_region.

TestSuite.h

Changed the implementation of ASSERT_CMPTIME and ASSERT_WITHIN_TIME_INTERVAL from using ASSERT_CMPINT to ASSERT_CMPINT64 for consistency with the return type of function bson_get_monotonic_type.

mock-rs.h

Changed the id parameter type for function mock_rs_elect from int to size_t.

request.h

Changed the n parameter type for function request_get_doc from int to size_t.

@eramongodb eramongodb requested a review from kkloberdanz April 5, 2023 16:31
Copy link
Contributor

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay on this one. LGTM.

@eramongodb eramongodb merged commit abb0bb7 into mongodb:master Apr 14, 2023
@eramongodb eramongodb deleted the cdriver-warnings branch April 14, 2023 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants