[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: |
|
||||||||
| 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? 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:
|
| 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: |
| 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: |
| 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. As it is now, the function is both future proof, and the documentations are correct. The return value should be checked. |