I suppose we should also cover the case where this is called before the call to mongoc_bulkwrite_execute. I can think of a few ways to handle this:

• Document the current behavior, which is to return false (simple, but potentially misleading)
• Do the same as above, but document as undefined behavior (also simple, but I generally prefer to specify behavior)
• Add BSON_ASSERT(self->executed) to the implementation (specifies behavior in a predictable way, but makes the error non-recoverable)
• Add a bson_error_t out-param to capture the error condition where self->executed is false (consistent with how other bulkwrite functions handle the case where self->executed is true, but seems like overkill)

Thoughts?

I like that idea. I would rather BSON_ASSERT(self->executed) or add a bson_error_t out-param. I slightly prefer a bson_error_t out-param for API consistency at the cost of a more verbose API.

I slightly prefer a bson_error_t out-param for API consistency at the cost of a more verbose API.

I also prefer this, but it will make this function signature a bit awkward. As far as I can tell, the API always uses bson_error_t out-params in conjunction with a bool or pointer return type, where false or NULL indicate the error is set. In this case, since we are already using the bool return to indicate whether or not the bulk write is acknowledged, it's not exactly clear how best to handle this.

We could use another bool out-param, but this makes it easy to confuse the meanings of the two bools:

MONGOC_EXPORT(bool)
mongoc_bulkwrite_is_acknowledged(mongoc_bulkwrite_t const *self, bool *error_is_set, bson_error_t *error);

We could use a tristate, and in that case I would also change the function's name slightly:

typedef enum {
 MONGOC_BULKWRITE_CHECK_ACKNOWLEDGED_ERROR = -1,
 MONGOC_BULKWRITE_UNACKNOWLEDGED,
 MONGOC_BULKWRITE_ACKNOWLEDGED,
} mongoc_bulkwrite_check_acknowledged_t;
MONGOC_EXPORT(mongoc_bulkwrite_check_acknowledged_t)
mongoc_bulkwrite_check_acknowledged(mongoc_bulkwrite_t const *self, bson_error_t *error);

Or a struct:

typedef struct {
 bool is_error;
 bool is_acknowledged;
} mongoc_bulkwrite_check_acknowledged_t;
MONGOC_EXPORT(mongoc_bulkwrite_check_acknowledged_t)
mongoc_bulkwrite_check_acknowledged(mongoc_bulkwrite_t const *self, bson_error_t *error);

I am just trying to pin down what the standard approach should be here, at the risk of bikeshedding.

As far as I can tell, the API always uses bson_error_t out-params in conjunction with a bool or pointer return type, where false or NULL indicate the error is set.

Yes, I think that is a widely used pattern in the C driver.

On a brief look at declarations marked BSON_EXPORT and MONGOC_EXPORT I do not see functions with bool out-params.

I like the tristate and struct return ideas. I think it simplifies the call (no need to separately declare a short-lived bool). The tristate return might introduce a slight inconsistency with the soon-to-be-added API to get the server ID in CDRIVER-5843. Maybe a struct return could be used for both:

typedef struct {
 bool is_error;
 bool is_acknowledged;
} mongoc_bulkwrite_acknowledged_t;
MONGOC_EXPORT(mongoc_bulkwrite_check_acknowledged_t)
mongoc_bulkwrite_check_acknowledged(mongoc_bulkwrite_t const *self, bson_error_t *error);
typedef struct {
 bool is_error;
 uint32_t server_id;
} mongoc_bulkwrite_serverid_t;
MONGOC_EXPORT(mongoc_bulkwrite_serverid_t)
mongoc_bulkwrite_get_serverid(mongoc_bulkwrite_t const *self, bson_error_t *error);

Good point about CDRIVER-5843 -- I'll need to change my implementation to account for the error case. I think the struct return is the best option for both.