Skip to content
Closed
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/odbc/php_odbc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1765,7 +1765,7 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
if (result_type & ODBC_NUM) {
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &tmp, sizeof(zval *), NULL);
} else {
if (!*(result->values[i].name)) {
if (!*(result->values[i].name) && Z_TYPE_P(tmp) != IS_NULL) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be Z_TYPE_P(tmp) == IS_STRING, rather than not IS_NULL? From context, it's obvious that the only two possible zval types for tmp are IS_STRING and IS_NULL, but this strikes me as a potential gotcha if the function is changed down the track to return more native PHP types for numeric values.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adam I think you are right about that. Changing to Z_TYPE_P(tmp) ==
IS_STRING seems like a better way to avoid the problem.

I will ask for help on IRC for how to submit a better fix.

Thanks,
Brandon Kirsch

On Wed, Sep 12, 2012 at 11:13 PM, Adam Harvey notifications@github.comwrote:

In ext/odbc/php_odbc.c:

@@ -1765,7 +1765,7 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type)
if (result_type & ODBC_NUM) {
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &tmp, sizeof(zval *), NULL);
} else {

  •       if (!*(result->values[i].name)) {
    
  •       if (!*(result->values[i].name) && Z_TYPE_P(tmp) != IS_NULL) {
    

Should this be Z_TYPE_P(tmp) == IS_STRING, rather than not IS_NULL? From
context, it's obvious that the only two possible zval types for tmp are
IS_STRING and IS_NULL, but this strikes me as a potential gotcha if the
function is changed down the track to return more native PHP types for
numeric values.


Reply to this email directly or view it on GitHubhttps://github.com//pull/193/files#r1594144.

zend_hash_update(Z_ARRVAL_P(return_value), Z_STRVAL_P(tmp), Z_STRLEN_P(tmp)+1, &tmp, sizeof(zval *), NULL);
} else {
zend_hash_update(Z_ARRVAL_P(return_value), result->values[i].name, strlen(result->values[i].name)+1, &tmp, sizeof(zval *), NULL);
Expand Down