hi!

On Mon, Sep 12, 2011 at 2:29 AM, Avi Brender <abren...@elitehosts.com> wrote:
> 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.

+       REGISTER_LONG_CONSTANT("FTP_USEPASVADDRESS",
PHP_FTP_OPT_USEPASVADDRESS, CONST_PERSISTENT | CONST_CS);

It is a userland constant too.

Everyone understands FTP_AUTORESUME or FTP_TIMEOUT_SEC,
_FTP_OPT_USEPASVADDRESS is cryptic compared to the other :)

> 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
> 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.

That's fine too. You can the tests with the patch too, just do svn add
ext/ftp/tests/... before you call svn diff.

Thanks for your work so far!

Cheers,
-- 
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to