Skip to content

ext/phar: avoid redundant allocation by using zend_string for alias#21785

Open
arshidkv12 wants to merge 4 commits intophp:masterfrom
arshidkv12:phar-5
Open

ext/phar: avoid redundant allocation by using zend_string for alias#21785
arshidkv12 wants to merge 4 commits intophp:masterfrom
arshidkv12:phar-5

Conversation

@arshidkv12
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

No technical review performed.

Comment thread ext/phar/zip.c Outdated
}

if (!actual_alias && zend_string_equals_literal(entry.filename, ".phar/alias.txt")) {
if (actual_alias == NULL && zend_string_equals_literal(entry.filename, ".phar/alias.txt")) {
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.

Please avoid changes like this. There are others in this PR.

Comment thread ext/phar/zip.c Outdated
mydata->is_temporary_alias = 0;

if (NULL != (fd_ptr = zend_hash_str_find_ptr(&(PHAR_G(phar_alias_map)), actual_alias, mydata->alias_len))) {
if (NULL != (fd_ptr = zend_hash_str_find_ptr(&(PHAR_G(phar_alias_map)), ZSTR_VAL(actual_alias), ZSTR_LEN(actual_alias)))) {
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.

Indentation is broken.

@iluuu1994
Copy link
Copy Markdown
Member

@Girgias Would you like to review this? I saw you make some phar PRs, I don't know if that means you're interested in getting more familiar with it?

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.

2 participants