Do not use assert() for checking OS API functions #40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mples Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
- Simplify code, no need for else if' when previous 'if' always returns - Add whitespace to sectionalize code, readability - Return NULL, not 0, for pointers - Remove useless assignment before free() Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
…location. This patch removes assert()'s used for checking memory allocation: calloc(), malloc(), strdup(). Instead libConfuse now returns NULL to allow the caller to handle such errors more gracefully. Some memory allocations did even have an assert(), they simply assumed memory had been successfully allocated. These have also been fixed. For internal functions which previously asserted, their return value is now verified by the public API's using them. Some minor cleanup is also included in this commit: - Readability: avoid assignment inside conditionals - Whitespace for sectionalizing code Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
troglobit
force-pushed
the
reduce-asserts
branch
from
06203dc
to
fe91f27
Compare
|
Force pushed update to last commit after insightful comment on |
After comment from @peda-r: - Handle strdup() failure in cfg_dupopt_array() - Handle strdup() failure in cfg_opt_setnstr() Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
…() [SQUASHME] The cfg_opt_getval() API now returns NULL, we must check for that now so we do not dereference invalid pointers. Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
…dation This patch introduces a semantic change in the behavior of libConfuse on invalid input to API's. Replacing assert() for argument validation with a simple if() statement (inverted logic compared to asserts) with errno set to EINVAL and returning cfg_false, NULL or similar on error. Some API's, mostly cfg_*set*(), are declared to return void, others, cfg_*getint/float/bool*(), return the actual data value. The former set functions will be changed in a later patch to return a status (POSIX OK(0) or non-zero on error), and the latter will likely return zero on error, which means the user will have to consult errno to check for error. Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
This patch changes the API for libConfuse set functions to return the status of the operation: CFG_FAIL, with errno set on error, and CFG_SUCCESS (POSIX OK) on success. Also, the API docs in confuse.h is updated with new @return lines. Signed-off-by: Joachim Nilsson <troglobit@gmail.com>
|
A few more changes related to issue #37 have now been added to this pull request.
|
|
I'll let this PR linger a bit more, pending comments from other devs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request is a mix of
.gitignoreupdates, doc comment fixes fromprevious pull request, whitespace and readability cleanup. But the most
important patch is the last one which replaces the use of
assert()as ameans of verifying OS memory allocation with NULL returns.