Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:111882
MIME-Version: 1.0
References: <20200917145304.4a6cfe45@mcmic-probook.opensides.be>
 <CAFPFaMKCoLuYfZfdXqG=tp5EkLoODuOjNPEB2HaLv_p54Ar=2Q@mail.gmail.com> <20200917152507.1203e8e2@mcmic-probook.opensides.be>
In-Reply-To: <20200917152507.1203e8e2@mcmic-probook.opensides.be>
Date: Thu, 17 Sep 2020 15:27:53 +0200
Message-ID: <CAFPFaMLOetBW+TU98WQNiPAL+yNLhX6FY6EnYJHOEO+uNt5B7w@mail.gmail.com>
To: =?UTF-8?Q?C=C3=B4me_Chilliet?= <come.chilliet@fusiondirectory.org>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a02aae05af825a06"
Subject: Re: [PHP-DEV] Stubs in ext/ldap
From: george.banyard@gmail.com ("G. P. B.")

--000000000000a02aae05af825a06
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

 On Thu, 17 Sep 2020 at 15:18, C=C3=B4me Chilliet <
come.chilliet@fusiondirectory.org> wrote:

> Le Thu, 17 Sep 2020 15:03:05 +0200,
> "G. P. B." <george.banyard@gmail.com> a =C3=A9crit :
> > A lot of the documentation is not up to date and will needs to be updat=
ed
> > 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 trus=
t.
>
> 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 functio=
n.
> >
> > > I would like to fix those conflicts.
> > > Basically my questions are:
> > > 1) What is the difference between s and S in zend_parse_parameters an=
d
> 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) !=3D
> 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 =3D UNKNOWN to =3D 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 nullabl=
e
> > 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 fro=
m
>  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=C3=B4me
>

George P. Banyard

--000000000000a02aae05af825a06--