Skip to content

ext/pdo_odbc: Do not populate message if there is no driver error #17478

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 15, 2025
Merged

ext/pdo_odbc: Do not populate message if there is no driver error #17478

merged 1 commit into from
Jan 15, 2025

Conversation

@Girgias
Copy link
Member

@Girgias Girgias commented Jan 15, 2025

@NattyNarwhal do you have instructions on how to set a DB on Linux/Fedora?

I think the double warning is an existing bug in PDO anyway, but I'm still confused as to why it is printing NULL bytes here.

@NattyNarwhal
Copy link
Member

For setting up SQL Server on Linux, I set up a VM or container running Ubuntu LTS (if you do that, use 22.04, 24.04 requires some work to make it run. The Docker container should also work as well.

@Girgias
Copy link
Member Author

Girgias commented Jan 15, 2025

Okay, "no need" to set-up a VM, I figured out the issue by staring at PDO long enough.

Only issue is I'm not exactly sure how to handle the following build failure on Linux X64 NTS:

 /home/runner/work/php-src/php-src/ext/pdo_odbc/odbc_driver.c: In function ‘pdo_odbc_fetch_error_func’:
/home/runner/work/php-src/php-src/ext/pdo_odbc/odbc_driver.c:44:33: error: the comparison will always evaluate as ‘false’ for the address of ‘last_err_msg’ will never be NULL [-Werror=address]
   44 |         if (einfo->last_err_msg == NULL || strlen(einfo->last_err_msg) == 0) {
      |                                 ^~
In file included from /home/runner/work/php-src/php-src/ext/pdo_odbc/odbc_driver.c:29:
/home/runner/work/php-src/php-src/ext/pdo_odbc/php_pdo_odbc_int.h:116:14: note: ‘last_err_msg’ declared here
  116 |         char last_err_msg[SQL_MAX_MESSAGE_LENGTH];
      |              ^~~~~~~~~~~~

As it seems that last_err_msg is not always a fixed length array, any suggestions?

@Girgias Girgias marked this pull request as ready for review January 15, 2025 15:54
@NattyNarwhal
Copy link
Member

I'm pretty sure last_err_msg should always be a fixed-length array in pdo_odbc_errorinfo, so the null check is redundant.

@Girgias Girgias force-pushed the odbc-pdo-driver-fix-null-pointer-as-string-print branch from c83954e to 722d0bd Compare January 15, 2025 19:11
@Girgias Girgias merged commit df8ac4a into php:master Jan 15, 2025
10 checks passed
@Girgias Girgias deleted the odbc-pdo-driver-fix-null-pointer-as-string-print branch January 15, 2025 20:46
Girgias added a commit to Girgias/php-src that referenced this pull request Jan 17, 2025
@Girgias
ext/pdo_odbc: Do not populate message if there is no driver error (ph…
e1eac15 
…p#17478)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NattyNarwhal NattyNarwhal NattyNarwhal approved these changes

@SakiTakamachi SakiTakamachi Awaiting requested review from SakiTakamachi SakiTakamachi is a code owner

Assignees

No one assigned

Labels

Extension: pdo (core) Extension: pdo_odbc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Girgias @NattyNarwhal