[CDRIVER-2095] Document what false means as return value Created: 23/Mar/17  Updated: 07/May/18  Resolved: 05/May/17

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: None
Fix Version/s: 1.7.0

Type: Improvement Priority: Minor - P4
Reporter: David Golden Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by CXX-1066 Audit all libmongoc and libbson calls... Closed

 Description   

The documentations never state what a false return value means. Does it mean a NULL was passed? Internal error, such as allocation errors? What sort of scenarios lead to a return value being false rather then true?
This needs to documented in one location and linked from functions with boolean return values

Following is the original report

The setters on mongoc_find_and_modify_opts_t like mongoc_find_and_modify_opts_set_sort and others generally return a bool and are documented as "Returns true if it successfully added the option to the builder."

There is no documentation of what "false" means. Looking at the code, some of them appear to have no way to return false. Others return false if their input is null. Without inspecting the code, we have no way of knowing if we should be checking the return value and without documentation, the behavior is unspecified and thus subject to change.

We are auditing all mongocxx calls to libmongoc to ensure mongocxx handles error conditions according to the libmongoc documentation. We can't do that if return value documentation isn't complete.

For all the setters, we would like the following changes to documentation:

  • document what 'false' means OR document that the function only ever returns true


 Comments   
Comment by Githook User [ 05/May/17 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-2095 allow NULL findandmodify options
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/82dbe605e91fb817fb56fa1954e3bf97a3a28b09

Comment by Githook User [ 05/May/17 ]

Author:

{u'username': u'ajdavis', u'name': u'A. Jesse Jiryu Davis', u'email': u'jesse@mongodb.com'}

Message: CDRIVER-2095 improve error-reporting docs
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/62e8fc399e9a3f23cca6493bf57d728d4d7bbf77

Comment by David Golden [ 20/Apr/17 ]

I think you misunderstand.

Right now, we don't know – without code inspection – whether failure means "something went wrong in libmongoc" (e.g. memory allocation error) or "the input was invalid" or both or neither. We'd like that documented so that we know what contract libmongoc is offering and thus we know what kind of error handling to invoke. E.g. abort() vs throw an "invalid parameter" exception.

Comment by Hannes Magnusson [ 20/Apr/17 ]

The functions state they return true on success, and therefore false on failure.

Today, there may be couple of functions that cannot fail and therefore don't return false, but updating the documentation to say "will never return false" is not going to happen as it is totally incorrect.

The exact handling today is an implementation detail.
Declaring the function as void will be equally bad, as when the day comes where something might fail, we can't change the prototype and we've shot ourself in the foot.

As it is now, the function is both future proof, and the documentations are correct. The return value should be checked.

Generated at Wed Feb 07 21:14:09 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.