Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:31477
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 83.243.58.163 as permitted sender)
To: David Soria Parra <dsp@php.net>
Cc: internals@lists.php.net
In-Reply-To: <FF.1D.61247.4C827B64@pb1.pair.com>
References: <FF.1D.61247.4C827B64@pb1.pair.com>
Content-Type: text/plain
Date: Tue, 07 Aug 2007 17:46:55 +0200
Message-ID: <1186501615.2772.38.camel@johannes.nop>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] Getopt with longopts and windows support -
	part 2
From: johannes@php.net (Johannes =?ISO-8859-1?Q?Schl=FCter?=)

David,

On Mon, 2007-08-06 at 09:57 -0400, David Soria Parra wrote:
> Hi internals,
> 
> so based on a discussion of php.pecl we thought about using the php_getopt
> for the PHP_FUNCTION(getopt) instead of the system provided getopt. Here
> is the patch for that. Hopefully someone is reviewing and commiting the
> patch as i don't have karma. 

The idea is good imo, but there are a few things we have to do:

- We need more tests, as far as I saw using grep we just have a single
  testcase for getopt() which is certainly not enough when replacing the
  implementation.

- The current implementation is not thread-safe, I don't know about the
  glibc implementation but if we use our own implementation for
  PHP's getopt function that should be threadsafe.

- If you move php_getopt to main you have to mark it as PHPAPI so the
  SAPIs can use it.

Maybe there's more, didn't do a deeper review of the patch, yet, since
I'd like to wait a) until Jani moved the files and b) we have more
tests :-)

johannes