Skip to content

Conversation

@kissifrot
Copy link

I can test a Windows build ogf PHP with that fix if needed.

It will probably need fixes/tweaks, especially with the given DSN at line 12
@kissifrot
Copy link
Author

Added test.

@cjbj
Copy link
Contributor

cjbj commented May 9, 2012

I scanned the change quickly. The code looks delicate. It defines the new size in terms of a utext (a short) which is intended for utf-16. If this sizing code is the root cause of the crash, I would have expected the patch to use oratext or text with some character set multiplication factor.

The test uses the wrong number of args to putenv() so it will fail. It hardcodes the connection string and char set. It should also have a skipif if the DB char set is not UTF8.

@kissifrot
Copy link
Author

Which patch?

@lstrojny
Copy link
Contributor

Can you squash the commit?

@gureedo
Copy link

gureedo commented Dec 11, 2013

It is not quite right solution.
Oracle docs defines three versions of utf8 encoding.

There is also another one caveat: NCHAR, NVARCHAR2, NCLOB types. Before transferring data to client Oracle converts data from such columns to NLS_NCHAR character set (if it is defined). Look here for details: http://docs.oracle.com/cd/A84870_01/doc/server.816/a76966/ch2.htm#94684
So, if NLS_NCHAR defined with different character set than connection charset or NLS_LANG, we can get invalid max character width for such columns.

I see here two ways.

  1. Always multiply data_size by 4, and not support old UTF8 (Unicode 3.0) character set.
  2. Detect character set and multiply by obtained max width.

But i don't know what to do with NLS_NCHAR.

@gitgitcode
Copy link

use oci function cast() like this :
select cast( tbl.description AS
varchar2(300) )as desc FROM database.table tbl;
it work 🎱

@cofirazak
Copy link

Can anyone clarify the status of this bug?
I've recompiled yestarday php with this fix to col->maxlen in oci_statement.c given by kissifrot, and it solved this bug.
So now i wonder why this bug is so old and still not merged in the master branch?
Can anyone clarify this please?

@php-pulls
Copy link

Comment on behalf of krakjoe at php.net:

Since this PR has merge conflicts, seems to have been abandoned by author, and seems like a questionable solution, I'm closing this PR.

If the author is watching and feels I am wrong, please open a clean PR, and start a discussion on internals.

@php-pulls php-pulls closed this Jan 1, 2017
@kissifrot
Copy link
Author

Well the original PR was more a quick hack than a real fix.
There is another PR and a different fix, #542 , about this

EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 25, 2025
       Comprehensive performance optimization of TrueAsync API to eliminate expensive EG(exception) checks in hot execution paths by refactoring function signatures from void to
       bool and replacing exception-based error handling with direct return value checks.

       Key changes:
       - Updated function typedefs: zend_async_event_del_callback_t, zend_async_event_stop_t, zend_async_event_dispose_t (void→bool)
       - Refactored zend_async_callbacks_remove to return bool
       - Updated all del_callback, stop, and dispose function implementations with proper return values
       - Fixed forward declarations compatibility issues
       - Optimized all function call sites to use if (!function()) return false; pattern instead of EG(exception) checks
       - Applied optimization to ~80 functions across coroutine, scope, curl, and libuv reactor modules

       Performance impact:
       Replaces expensive exception checks with fast boolean operations in critical async execution paths.
EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 25, 2025
       Changed return types from void to bool for three functions to match header declarations:

       zend_async_internal_context_set:
       - Returns false when coroutine == NULL
       - Returns true on successful context setting

       zend_coroutine_call_switch_handlers:
       - Returns true when no handlers exist or execution succeeds
       - Preserves existing handler cleanup logic

       zend_async_add_main_coroutine_start_handler:
       - Returns false when duplicate handler detected
       - Returns true on successful handler addition
EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 25, 2025
EdmondDantes added a commit to true-async/php-src that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants