Skip to content

use smart_str_append() if we have a zend_string*#21414

Merged
Girgias merged 1 commit intophp:masterfrom
Girgias:smart-str-zstr
Mar 21, 2026
Merged

use smart_str_append() if we have a zend_string*#21414
Girgias merged 1 commit intophp:masterfrom
Girgias:smart-str-zstr

Conversation

@Girgias
Copy link
Copy Markdown
Member

@Girgias Girgias commented Mar 11, 2026

No description provided.

Comment thread Zend/zend_exceptions.c
case IS_OBJECT: {
zend_string *class_name = Z_OBJ_HANDLER_P(arg, get_class_name)(Z_OBJ_P(arg));
smart_str_appends(str, "Object(");
/* cut off on NULL byte ... class@anonymous */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/* cut off on NULL byte ... class@anonymous */
/* cut off on NUL byte ... class@anonymous */

Prefer NUL when referring to the (char *)0.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is copied from an existing comment and people always seem to disagree between NULL and NUL byte... So I'll leave it like this and someone can either try to codify or fix this across the whole codebase.

Comment thread Zend/zend_exceptions.c
@Girgias Girgias marked this pull request as ready for review March 16, 2026 14:18
Copy link
Copy Markdown
Member

@DanielEScherzer DanielEScherzer left a comment

Choose a reason for hiding this comment

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

Okay for ext/reflection

Copy link
Copy Markdown
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

intl ok

@Girgias Girgias merged commit f40b356 into php:master Mar 21, 2026
19 checks passed
@Girgias Girgias deleted the smart-str-zstr branch March 21, 2026 17:06
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.

5 participants