On Thu, 17 Sep 2020 at 15:18, Côme Chilliet < come.chill...@fusiondirectory.org> wrote:
> Le Thu, 17 Sep 2020 15:03:05 +0200, > "G. P. B." <george.bany...@gmail.com> a écrit : > > A lot of the documentation is not up to date and will needs to be updated > > after > > there has been a check of the argument names for consistency with other > > extensions in regards to named params, the stubs are the source of trust. > > I think a review of ext/ldap is needed then. > https://github.com/php/php-tasks/issues/23 There is a tracker about extensions which needs to have a review still: > > UNKNOWN means that the default value cannot be specified due to some > reason > > mostly the case for arguments passed by references or funcky function > > signature > > implementation which check the number of argument passed to the function. > > > > > I would like to fix those conflicts. > > > Basically my questions are: > > > 1) What is the difference between s and S in zend_parse_parameters and > how > > > do > > > they behave for a NULL value? > > > > None in regards to userland as 's' is a char* followed by a size_t ad 'S' > > is a zend_string. > > As for all internal functions which are not nullable they will coerce > null > > to the respective type > > so for a string argument null would become an empty string. > > So in the example of ldap_bind: > if (zend_parse_parameters(ZEND_NUM_ARGS(), "r|ss", &link, > &ldap_bind_dn, &ldap_bind_dnlen, &ldap_bind_pw, &ldap_bind_pwlen) != > SUCCESS) > { RETURN_THROWS(); } > > With ldap_bind($link, NULL), ldap_bind_dn is not NULL but an empty string? > Exactly as Tyson also just explained > > 2) What would changing = UNKNOWN to = NULL impact? > > > > Explicit support for nullable types in the C implementation > > > > > 3) When a function like ldap_bind is called, is calling > ldap_bind($link), > > > and > > > ldap_bind($link, NULL) changing anything else than ZEND_NUM_ARGS? > > > > This relates to point 1 and 2 above, if there is no support for nullable > > types > > passing null can change the meaning of it. > > Then I think most of these needs to be changed to NULL with explicit > support, > as it was most likely what was intended. (the underlying C functions from > libldap accepts NULL). > This should make it very easy then just need to add the '!' specifier to say the argument is nullable and it would set the char* to NULL if I'm not mistaken > Also, there are tests like ldap_sasl_bind_basic.phpt which tests a NULL > parameter. > > What is the dead line for this kind of changes in ext/ldap regarding to > PHP-8? > As we added another beta version before RC you have at least another week and a half for sure. However, I expect getting rid of UNKNOWN values can also be done in an RC release whereas naming should probably be fixed before RC1 comes out. > Côme > George P. Banyard
