Add error handling to Python API calls in LDAPraise_for_message #341
Conversation
Codecov Report
@@ Coverage Diff @@
## master #341 +/- ##
==========================================
- Coverage 71.30% 68.29% -3.02%
==========================================
Files 50 49 -1
Lines 4796 4861 +65
Branches 802 820 +18
==========================================
- Hits 3420 3320 -100
- Misses 1045 1187 +142
- Partials 331 354 +23
Continue to review full report at Codecov.
|
|
It'll review the PR next week or the week after. Too much C code for a Friday afternoon! |
| PyErr_SetObject(errobj, info); | ||
| Py_DECREF(info); | ||
| ldap_memvfree((void **)refs); | ||
| PyErr_SetObject(errobj, info); |
There was a problem hiding this comment.
I get a segfault here:
test105_reconnect_restore (Tests.t_ldapobject.Test01_ReconnectLDAPObject) ... make: *** [Makefile:72: valgrind] Segmentation fault (core dumped)
…_message - Ensure pointers are set to NULL when freed. - Free all non-NULL pointers at the end. - Use goto to jump to the cleanup. Note that Python API sets the current exception on failure. If something else than C API fails, PyErr_NoMemory or PyErr_SetObject is called before returning NULL. I recommend reviewing the diff with whitespace changes hidden (`-w` in Git CLI).
| if (m != NULL) { | ||
| msgid = ldap_msgid(m); | ||
| msgtype = ldap_msgtype(m); | ||
| ldap_parse_result(l, m, &errnum, &matched, &error, &refs, |
There was a problem hiding this comment.
The function call is missing an error check:
Upon success LDAP_SUCCESS is returned. Otherwise the values of the result parameters are undefined.
There was a problem hiding this comment.
Does that mean they should not be freed, like this?
opt_errnum = ldap_parse_result(l, m, &errnum, &matched, &error,
&refs, &serverctrls, 1);
if (opt_errnum != LDAP_SUCCESS) {
// values of the result parameters are undefined; zero them
matched = NULL;
error = NULL;
refs = NULL;
serverctrls = NULL;
errnum = opt_errnum;
}
Add CPython style error handling to
LDAPraise_for_message: ensure pointers are set to NULL when freed, and free all non-NULL pointers at the end. Usegototo jump to the cleanup. (The label is usually callederror, which here clashes with a variable name).Note that Python API sets the current exception on failure. If something else than C API fails,
PyErr_NoMemoryorPyErr_SetObjectis called before returning NULL.I recommend reviewing the diff with whitespace changes hidden (
-win Git CLI,?w=1in GitHub URLs).