Skip to content

Zend: Remove unused parameter from zend_dval_to_lval_cap() #21112

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

Open
nyrzhun wants to merge 1 commit into
base: master
Choose a base branch
from
+11 −10
Open

Zend: Remove unused parameter from zend_dval_to_lval_cap() #21112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 0 deletions UPGRADING.INTERNALS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ PHP 8.6 INTERNALS UPGRADE NOTES
zend_enum_RoundingMode parameter.
. Added Z_PARAM_ENUM().
. Added zend_enum_fetch_case_id().
. The zend_dval_to_lval_cap() function no longer takes a second
zend_string* parameter.

========================
2. Build system changes
Expand Down
13 changes: 6 additions & 7 deletions Zend/zend_operators.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,18 +427,17 @@ static zend_never_inline zend_long ZEND_FASTCALL zendi_try_get_long(const zval *
* We use use saturating conversion to emulate strtol()'s
* behaviour.
*/
if (op_str == NULL) {
/* zend_dval_to_lval_cap() can emit a warning so always do the copy here */
op_str = zend_string_copy(Z_STR_P(op));
}
lval = zend_dval_to_lval_cap(dval, op_str);
lval = zend_dval_to_lval_cap(dval);
if (!zend_is_long_compatible(dval, lval)) {
if (op_str == NULL) {
Copy link
Member

@ndossche ndossche Feb 3, 2026

Choose a reason for hiding this comment

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

Do we still need to make a copy? I believe not?

Copy link
Member

@iluuu1994 iluuu1994 Feb 3, 2026

Choose a reason for hiding this comment

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

There's an existing copy above, including a warning. So I think yes. Technically not here, but then we need to know whether to free or not, which is likely not worth the trouble.

Copy link
Member

@ndossche ndossche Feb 3, 2026

Choose a reason for hiding this comment

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

I still don't understand. zend_tmp_string_release already checks whether we need to free.
zend_incompatible_string_to_long_error triggers a warning that uses op as a string, even if that invalidates op, then the warning will have already been triggered.

Copy link
Member

@ndossche ndossche Feb 3, 2026

Choose a reason for hiding this comment

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

i.e. you could do zend_incompatible_string_to_long_error(op_str ? op_str : Z_STR_P(op));

Copy link
Member

@iluuu1994 iluuu1994 Feb 3, 2026

Choose a reason for hiding this comment

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

Ah, that's what you mean. Would also be fine by me.

op_str = zend_string_copy(Z_STR_P(op));
}
zend_incompatible_string_to_long_error(op_str);
if (UNEXPECTED(EG(exception))) {
*failed = true;
}
}
zend_string_release(op_str);
zend_tmp_string_release(op_str);
return lval;
}
}
Expand Down Expand Up @@ -994,7 +993,7 @@ ZEND_API zend_long ZEND_FASTCALL zval_get_long_func(const zval *op, bool is_stri
* behaviour.
*/
/* Most usages are expected to not be (int) casts */
lval = zend_dval_to_lval_cap(dval, Z_STR_P(op));
lval = zend_dval_to_lval_cap(dval);
if (UNEXPECTED(is_strict)) {
if (!zend_is_long_compatible(dval, lval)) {
zend_incompatible_string_to_long_error(Z_STR_P(op));
Expand Down
2 changes: 1 addition & 1 deletion Zend/zend_operators.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ static zend_always_inline zend_long zend_dval_to_lval_silent(double d)
}

/* Used to convert a string float to integer during an (int) cast */
static zend_always_inline zend_long zend_dval_to_lval_cap(double d, const zend_string *s)
static zend_always_inline zend_long zend_dval_to_lval_cap(double d)
{
if (UNEXPECTED(!zend_finite(d))) {
return 0;
Expand Down
2 changes: 1 addition & 1 deletion ext/dom/php_dom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2267,7 +2267,7 @@ static bool dom_nodemap_or_nodelist_process_offset_as_named(zval *offset, zend_l
if (0 == (is_numeric_string_type = is_numeric_string(Z_STRVAL_P(offset), Z_STRLEN_P(offset), lval, &dval, true))) {
return true;
} else if (is_numeric_string_type == IS_DOUBLE) {
*lval = zend_dval_to_lval_cap(dval, Z_STR_P(offset));
*lval = zend_dval_to_lval_cap(dval);
}
} else {
*lval = zval_get_long(offset);
Expand Down
2 changes: 1 addition & 1 deletion ext/tidy/tidy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ static bool php_tidy_set_tidy_opt(TidyDoc doc, const char *optname, zval *value,
}
uint8_t type = is_numeric_string(ZSTR_VAL(str), ZSTR_LEN(str), &lval, &dval, true);
if (type == IS_DOUBLE) {
lval = zend_dval_to_lval_cap(dval, str);
lval = zend_dval_to_lval_cap(dval);
type = IS_LONG;
}
if (type == IS_LONG) {
Expand Down
Loading