ext/ldap: Various refactorings to php_ldap_do_search()
#16104
Conversation
Girgias
force-pushed
the
ldap-list-search-refacto
branch
2 times, most recently
from
d69827a to
a99be0a
Compare
|
|
||
| zend_ulong attribute_index = 0; | ||
| zval *attribute_zv = NULL; | ||
| ZEND_HASH_FOREACH_NUM_KEY_VAL(attributes, attribute_index, attribute_zv) { |
There was a problem hiding this comment.
I'm pretty certain that references are not handled here, which results in a strange must be a list of strings, string given (probably similarly for other foreaches).
There was a problem hiding this comment.
The thing is I tried adding a test as I was thinking the same, and yet it seems to be handled.
I don't really get it, like the only thing which would make some sense is if separating the array would also deref the entries, but that seems improbable.
There was a problem hiding this comment.
Ahhh wait this is about the attributes and not the filter/base_dn arrays.
I did miss this will check.
There was a problem hiding this comment.
Well I added a test, and indeed it does work (which is very confusing to me)
There was a problem hiding this comment.
@Girgias That's not how you create a reference in an array, dump that into debug_zval_dump and you'll see these aren't references.
…Ps is not a list) Closes phpGH-16102
Girgias
force-pushed
the
ldap-list-search-refacto
branch
from
a99be0a to
1df3ca8
Compare
I am faily certain this was just leaking memory
Girgias
force-pushed
the
ldap-list-search-refacto
branch
from
1df3ca8 to
e205514
Compare
There is much more to be done here, such as checking the PHP
intinputs fit into a Cint(and possibly are >= 0, but that would require changing the default value from-1to0), turning the$attributes_onlyparameter into abool, and possibly just split the batch processing functionality into new functions.However, this should already address some issues and improve code readability.
Depends partially on: #16102