Skip to content

Avoid double allocation when joining To/Cc headers#58

Closed
iliaal wants to merge 2 commits into
php:masterfrom
iliaal:pr/header-join-concat
Closed

Avoid double allocation when joining To/Cc headers#58
iliaal wants to merge 2 commits into
php:masterfrom
iliaal:pr/header-join-concat

Conversation

@iliaal

@iliaal iliaal commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Folding repeated To:/Cc: headers built a throwaway buffer with emalloc + strcpy + two strcat calls, then handed it to add_assoc_string, which copied it again into a zend_string — two allocations and two copies per fold. Build the joined value once, directly into a zend_string via memcpy, and store it with add_assoc_str, which takes ownership. One allocation, one copy; output is unchanged.

Folding repeated To:/Cc: headers built a throwaway buffer with
emalloc + strcpy + two strcat calls, then handed it to
add_assoc_string, which copied it again into a zend_string before the
temporary was freed. That is two allocations and two copies per fold.

Build the joined value directly into a single zend_string with memcpy
and store it with add_assoc_str, which takes ownership. One allocation,
one copy. Output ("a, b, c") is unchanged.
Comment thread php_mailparse_mime.c Outdated
header_val is null-terminated, so copying add_len + 1 bytes pulls in its
NUL and makes the separate ZSTR_VAL(joined)[...] = '\0' assignment
redundant. zend_string_alloc already reserves the +1 byte for it.

Suggested by @remicollet in review of phpGH-58.
remicollet pushed a commit that referenced this pull request Jun 13, 2026
header_val is null-terminated, so copying add_len + 1 bytes pulls in its
NUL and makes the separate ZSTR_VAL(joined)[...] = '\0' assignment
redundant. zend_string_alloc already reserves the +1 byte for it.

Suggested by @remicollet in review of GH-58.
@remicollet

Copy link
Copy Markdown
Member

Merged, thanks

@remicollet remicollet closed this Jun 13, 2026
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.

2 participants