Skip to content

Fix missing and inconsistent error check on SQLAllocHandle #10740

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

Closed

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Mar 1, 2023

  • Missing check: SQLAllocHandle() for the environment wasn't checked in pdo_odbc_handle_factory(). Add a check similar to the other ones for SQLAllocHandle().
  • Inconsistent check: one of the SQLAllocHandle() calls wasn't checked for SQL_SUCCESS_WITH_INFO. However, looking at the other uses and the documentation we should probably check this as well.

Furthermore, since there was a mix of "SQLAllocHandle: reason" and "SQLAllocHandle (reason)" in the error reporting, I made them consistently use the first option as that seems to be the most used for error reporting in this file.

Found using an experimental static analyser I'm developing.

* Missing check: SQLAllocHandle() for the environment wasn't checked in
  pdo_odbc_handle_factory(). Add a check similar to the other ones for
  SQLAllocHandle().
* Inconsistent check: one of the SQLAllocHandle() calls wasn't checked
  for SQL_SUCCESS_WITH_INFO. However, looking at the other uses and the
  documentation we should probably check this as well.

Furthermore, since there was a mix of "SQLAllocHandle: reason" and
"SQLAllocHandle (reason)" in the error reporting, I made them
consistently use the first option as that seems to be the most used for
error reporting in this file.
@Girgias Girgias self-assigned this Mar 3, 2023
@nielsdos nielsdos assigned nielsdos and unassigned Girgias Mar 14, 2023
@nielsdos nielsdos closed this in c4c8d6c Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants