Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55380
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.160.170 as permitted sender)
MIME-Version: 1.0
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 11:54:22 +0200
Message-ID: <CAEZPtU6vepCA8Yu=3B8i6ha77DJAUxSBTMLBSFb=jFrNuzrybA@mail.gmail.com>
To: Avi Brender <abrender@elitehosts.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [PATCH]#55651
From: pierre.php@gmail.com (Pierre Joye)

hi!

On Mon, Sep 12, 2011 at 2:29 AM, Avi Brender <abrender@elitehosts.com> wrot=
e:
> 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.

+	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 ensu=
re
> 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 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 =A0so o=
nce
> 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,
--=20
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org