Skip to content

ext/ldap: fix ldap_explode_dn result ordering and simplify count handling#22519

Open
arshidkv12 wants to merge 3 commits into
php:masterfrom
arshidkv12:ldap_explode_dn
Open

ext/ldap: fix ldap_explode_dn result ordering and simplify count handling#22519
arshidkv12 wants to merge 3 commits into
php:masterfrom
arshidkv12:ldap_explode_dn

Conversation

@arshidkv12

Copy link
Copy Markdown
Contributor

@come-nc come-nc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me, basically a revert of 30bc98c

@Girgias Girgias left a comment

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.

Just move the add_assoc_long() call prior to the for loop

@Girgias

Girgias commented Jul 2, 2026

Copy link
Copy Markdown
Member

Also needs to target a lower branch.

Comment thread ext/ldap/ldap.c
array_init(return_value);
int i;

for (i = 0; ldap_value[i] != NULL; i++);

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.

What?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm a bit confused. Could you please clarify what you mean?

Is the following correct?

	add_assoc_long(return_value, "count", 0);

	for (i = 0; ldap_value[i] != NULL; i++) {
		add_index_string(return_value, i, ldap_value[i]);
	}
	add_assoc_long(return_value, "count", i);

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.

3 participants