-
Notifications
You must be signed in to change notification settings - Fork 103
Issue 6057 - Fix3 - Fix covscan issues #6127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| */ | ||
| slapi_log_err(SLAPI_LOG_ERR, "dbmdb_open_all_files", | ||
| "Unable to open the database environment witout either the database context or a backend.\n"); | ||
| return DBI_RC_INVALID; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you may use the famous dbmdb_map_error(_FUNCTION, DBI_RC_INVALID) to log the backtrace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, but IMHO the backtrace is useless and I prefer avoiding using a sledgehammer to break a nut! 😉
The only case to trigger the failure is either that memory get corrupted or that we added new broken code
(So far the only case dbmdb_open_all_files is called with a NULL backend is at startup when setting up the database environment context and in that case the context that was just created is provided.
In the other case (typically when a backend is (re)started the backend is provided)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit subject looks a bit strange - "Issue i6057 - Fix3 - Fix covscan issues".
Besides that, LGTM!
I fixed the typo in the issue number but other wise it seems ok as it is the third PR about 6057 issue |
Fix two minor issues reported by covscan after the previews fix: CID 1540758: Null pointer dereferences - NULL_RETURNS /ldap/servers/slapd/back-ldbm/vlv.c: 412 in vlv_list_filenames Generate Null pointer exception if vlv config entry is not compliant to the schema Added a ternary test to harden the code. CID 1540757: Null pointer dereferences - FORWARD_NULL /ldap/servers/slapd/back-ldbm/db-mdb/mdb_instance.c: 377 in dbmdb_open_all_files covscan complain that be may be null (which is true but not in the case database context is also NULL) Added a test to avoid the warning Issue #6057 Reviewed by: @tbordaz, @droideck Thanks!
Fix two minor issues reported by covscan after the previews fix: CID 1540758: Null pointer dereferences - NULL_RETURNS /ldap/servers/slapd/back-ldbm/vlv.c: 412 in vlv_list_filenames Generate Null pointer exception if vlv config entry is not compliant to the schema Added a ternary test to harden the code. CID 1540757: Null pointer dereferences - FORWARD_NULL /ldap/servers/slapd/back-ldbm/db-mdb/mdb_instance.c: 377 in dbmdb_open_all_files covscan complain that be may be null (which is true but not in the case database context is also NULL) Added a test to avoid the warning Issue #6057 Reviewed by: @tbordaz, @droideck Thanks! (cherry picked from commit 23a094c)
Fix two minor issues reported by covscan after the previews fix:
/ldap/servers/slapd/back-ldbm/vlv.c: 412 in vlv_list_filenames
Generate Null pointer exception if vlv config entry is not compliant to the schema
Added a ternary test to harden the code.
/ldap/servers/slapd/back-ldbm/db-mdb/mdb_instance.c: 377 in dbmdb_open_all_files
covscan complain that be may be null (which is true but not in the case database context is also NULL)
Added a test to avoid the warning
Issue #6057
Reviewed by: @tbordaz, @droideck Thanks!