Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55376
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.170 as permitted sender)
MIME-Version: 1.0
Sender: laruence@gmail.com
In-Reply-To: <4E6D5263.60509@elitehosts.com>
References: <4E6B9466.6060608@elitehosts.com> <CABBJUpcPqMtjhjBiaDU=TaOOqCM6o0sdjDgjkJ0BDi_dmnP5jQ@mail.gmail.com>
 <CAEZPtU4_KBWd=82-svSeNH+-U=xGiLLhXTc52HT9=Sy2dunnYA@mail.gmail.com>
 <4E6D1340.6010408@elitehosts.com> <CAEZPtU6k_-9cEeGvnDzVcBP0xZ68aptUMhEqfc=CPJqsxt8p8Q@mail.gmail.com>
 <4E6D5263.60509@elitehosts.com>
Date: Mon, 12 Sep 2011 12:29:30 +0800
Message-ID: <CABBJUpcm6pnA3FbRz1_446ioruyq12jSpERWOG-GOgBi=16H1g@mail.gmail.com>
To: Avi Brender <abrender@elitehosts.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [PATCH]#55651
From: laruence@php.net (Laruence)

2011/9/12 Avi Brender <abrender@elitehosts.com>:
> Hi,
>
> Please see if the attached patch better addresses your concerns.
>
> Regarding the variable name, the PHP_FTP_OPT_USEPASVADDRESS is only inter=
nal
> and is modeled after the other variables PHP_FTP_TIMEOUT_SEC and
> PHP_FTP_OPT_AUTOSEEK. The variable actually passed to the ftp_set_option(=
)
> function by users is FTP_USEPASVADDRESS as defined in php_ftp.c which is
> inline with the other variables FTP_AUTORESUME, FTP_TIMEOUT_SEC,
> FTP_AUTOSEEK etc.
>
> In terms of tests, what type of tests were you thinking of? We can't ensu=
re
> that ftp->pasvaddr is set properly in response to a PASV command unless
actually I think it can,  plz refer to the existing test config script
"server.inc" under the ftp/tests/,
and maybe you can simulate a test environ?

and thanks for your work on PHP
> there's a way to expose those internal variables to the test - I'm not
> familiar enough with the internal PHP code to know if that's possible. If
> you're referring to tests that ensure that 1/0 true/false values passed t=
o
> ftp_set_option() work properly then I've attached a test file for that.
>
> I don't want to clutter the bugzilla ticket with many attachments =C2=A0s=
o once
> the patch is approved I will post the final version in the ticket if that=
's
> okay.
>
> All the best,
>
> Avi Brender
> Elite Hosts, Inc
> www.elitehosts.com
> ------------------------------------------------
> WARNING !!! This email message is for the sole use of the intended
> recipient(s) and may contain confidential and privileged information. Any
> unauthorized review; use, disclosure or distribution is prohibited, and
> could result in criminal prosecution. If you are not the intended recipie=
nt,
> please contact the sender by reply email and destroy all copies of the
> original message. This message is private and is considered a confidentia=
l
> exchange - public disclosure of this electronic message or its contents a=
re
> prohibited.
>
>
> On 09/11/2011 04:24 PM, Pierre Joye wrote:
>>
>> hi!
>>
>> Please upload the patch in the bug tracker as well.
>>
>> It would be also better to use a more verbose name.
>> FTP_OPT_USEPASVADDRESS is somehow cryptic.
>>
>> Laruence's comment is still valid, the zval should be converted if it
>> is not int or bool.
>>
>> Btw, could you test cases as well please?
>>
>> Cheers,
>>
>> On Sun, Sep 11, 2011 at 10:00 PM, Avi Brender<abrender@elitehosts.com>
>> =C2=A0wrote:
>>>
>>> Hi,
>>>
>>> I've updated the patch - please see attached.
>>>
>>> Avi Brender
>>> Elite Hosts, Inc
>>> www.elitehosts.com
>>> ------------------------------------------------
>>> WARNING !!! This email message is for the sole use of the intended
>>> recipient(s) and may contain confidential and privileged information. A=
ny
>>> unauthorized review; use, disclosure or distribution is prohibited, and
>>> could result in criminal prosecution. If you are not the intended
>>> recipient,
>>> please contact the sender by reply email and destroy all copies of the
>>> original message. This message is private and is considered a
>>> confidential
>>> exchange - public disclosure of this electronic message or its contents
>>> are
>>> prohibited.
>>>
>>> On 09/11/2011 10:53 AM, Pierre Joye wrote:
>>>>
>>>> hi,
>>>>
>>>> A simple test if it is IS_BOOL or IS_LONG should be enough, both types
>>>> use the the long value (convert_to_boolean_ex is slow and duplicate
>>>> the zval while it is not necessary). Then test> =C2=A00 instead of sim=
ply
>>>> assigning the value.
>>>>
>>>> On Sun, Sep 11, 2011 at 5:59 AM, Laruence<laruence@php.net> =C2=A0wrot=
e:
>>>>>
>>>>> Hi:
>>>>> =C2=A0 =C2=A0after a quick look, =C2=A0I have one suggestion,
>>>>> =C2=A0 =C2=A0if the (Z_TYPE_P(z_value) !=3D IS_BOOL), you should call
>>>>> convert_to_boolean_ex to convert it to a boolean
>>>>> =C2=A0 =C2=A0otherwise, people can not use a interge 1 as a true flag=
.
>>>>>
>>>>> =C2=A0 =C2=A0thanks
>>>>>
>>>>> 2011/9/11 Avi Brender<abrender@elitehosts.com>:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I've submitted bug #55651 along with a patch to implement a fix (als=
o
>>>>>> attached) for the passive FTP mode issue. I was hoping that someone
>>>>>> could
>>>>>> review the bug report and consider the patch for inclusion in future
>>>>>> PHP
>>>>>> releases.
>>>>>>
>>>>>> Thanks so much!
>>>>>>
>>>>>> Avi Brender
>>>>>> Elite Hosts, Inc
>>>>>> www.elitehosts.com
>>>>>>
>>>>>> --
>>>>>> PHP Internals - PHP Runtime Development Mailing List
>>>>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Laruence =C2=A0Xinchen Hui
>>>>> http://www.laruence.com/
>>>>>
>>>>> --
>>>>> PHP Internals - PHP Runtime Development Mailing List
>>>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>>>
>>>>>
>>>>
>>>>
>>>
>>> --
>>> PHP Internals - PHP Runtime Development Mailing List
>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>
>>
>>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>



--=20
Laruence =C2=A0Xinchen Hui
http://www.laruence.com/