Skip to content

Conversation

@kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Mar 31, 2025

Summary

Apply missing BSON_CALL to all exported callbacks to ensure __cdecl is applied when building with MSVC.

Resolves CDRIVER-2737 and CDRIVER-5678.

Verified with this patch build.

Background & Motivation

Existing expectations

This PR assumes libbson and libmongoc do not support building with a non-cdecl calling convention. Attempting to build libbson with /Gv to specify the __vectorcall calling convention results in errors:

src\libbson\src\bson\bson-clock.c(45): error C2373: 'bson_gettimeofday': redefinition; different type modifier
(... and many more ...)

This PR intends to ensure consumers building with MSVC see __cdecl in the function declaration when building headers to match the expected calling convention of libmongoc and libbson.

Skipping CI testing

Similar to mongodb/mongo-cxx-driver#1202, an attempt was made to build test-libmongoc with /Gv to test a non-cdecl consumer.

Doing so expectedly appears to require marking all callback functions defined in tests with __cdecl to avoid errors like:

src\libmongoc\tests\test-mongoc-command-monitoring.c(131): error C2440: 'function': cannot convert from 'void (__vectorcall *)(const mongoc_apm_command_failed_t *)' to 'mongoc_apm_command_failed_cb_t'
(... and many more ...)

And also appears to require marking functions in private headers used by tests to be marked __cdecl:

test-libmongoc-lib.lib(runner.obj) : error LNK2001: unresolved external symbol _mongoc_array_append_vals@@24
(... and many more ...)

To expedite the fix, tests in CI are not added.

Add `BSON_CALL` to public callbacks to support consumers building with non-cdecl calling convention.
libmongoc and libbson are expected to only be built with cdecl. Mark public API with cdecl to tell consumers.
@kevinAlbs kevinAlbs requested a review from eramongodb March 31, 2025 12:56
Copy link
Contributor

@eramongodb eramongodb left a comment

Choose a reason for hiding this comment

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

The pointer-to-function data members in mongoc_stream_t also require __cdecl.

Copy link
Collaborator Author

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

The pointer-to-function data members in mongoc_stream_t also require __cdecl.

Great catch. Updated.

@kevinAlbs kevinAlbs requested a review from eramongodb March 31, 2025 15:59
@kevinAlbs kevinAlbs merged commit 0c3c81b into mongodb:master Mar 31, 2025
37 of 41 checks passed
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.

2 participants