2011/9/12 Avi Brender <abren...@elitehosts.com>: > Hi, > > Please see if the attached patch better addresses your concerns. > > Regarding the variable name, the PHP_FTP_OPT_USEPASVADDRESS is only internal > 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 ensure > 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 to > 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 so 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 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 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<abren...@elitehosts.com> >> wrote: >>> >>> 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. Any >>> 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> 0 instead of simply >>>> assigning the value. >>>> >>>> On Sun, Sep 11, 2011 at 5:59 AM, Laruence<larue...@php.net> wrote: >>>>> >>>>> Hi: >>>>> after a quick look, I have one suggestion, >>>>> if the (Z_TYPE_P(z_value) != IS_BOOL), you should call >>>>> convert_to_boolean_ex to convert it to a boolean >>>>> otherwise, people can not use a interge 1 as a true flag. >>>>> >>>>> thanks >>>>> >>>>> 2011/9/11 Avi Brender<abren...@elitehosts.com>: >>>>>> >>>>>> Hi, >>>>>> >>>>>> I've submitted bug #55651 along with a patch to implement a fix (also >>>>>> 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 Xinchen 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 > -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
