17 years ago by Felipe Pena — view source

unread

Hello

Em Qui, 2008-06-19 às 16:06 -0700, Stanislav Malyshev escreveu:

Hi!

While we nearing the release of 5.3 (hopefully?), there are many
functions in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

Yeah, I have noticed... That's also in my TODO.

--
Regards,
Felipe Pena.

17 years ago by Philip Olson — view source

unread

While we nearing the release of 5.3 (hopefully?), there are many
functions in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one
(zend_parse_parameters).

Yeah, I have noticed... That's also in my TODO.

Where is the Felipe fan club signup sheet?

Regards,
Philip

17 years ago by David Coallier — view source

unread

I'll take a few

2008/6/20 Stanislav Malyshev stas@zend.com:

Hi!

While we nearing the release of 5.3 (hopefully?), there are many functions
in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

I have cleaned up Zend engine functions, converting them to the new API,
but there are about 1000 instances throughout PHP code (especially
ext/standard) which still use the old way. This way is less correct,
inconsistent with the new code, gives worse error messages, more prone to
various bugs, etc. All new extensions and functions use the new way, and the
only reason for keeping the old way in the code seems to be that nobody
cleaned it up. Somebody has to bring the old ones into sync, and I don't
think I personally will have time to do it in near timeframe. So if anybody
could step up to that - by doing at least part of the work - that'd be
great. The work is pretty simple, taking something like:

zval **data;

if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &data) == FAILURE) {
WRONG_PARAM_COUNT;
}
convert_to_string_ex(data);

and move it into:

char *str;
int str_len;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &str, &str_len)
== FAILURE) {
return;
}

and of course convert use of data zval into use of str.

This needs to be done carefully as not to break things on some more tricky
functions which may get arguments of multiple types, etc. Also, some tests
which check error messages may need to be fixed since new API gives more
detailed messages.
zend_parse_parameters documented here:
http://www.php.net/manual/en/internals2.ze1.zendapi.php
though some of the newer parameters aren't (C, h, f, Z) so one may need to
look at the code at zend_API.c.

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

--

--
Slan,
David

17 years ago by Olivier Hill — view source

unread

Want some help D? :)

Olivier

I'll take a few

17 years ago by David Coallier — view source

unread

2008/6/20 Olivier Hill ohill@php.net:

Want some help D? :)

Olivier

I'll take a few

Yep, after speaking with felipe, we (you and I) have standard/

--
Slan,
David

17 years ago by Olivier Hill — view source

unread

Stan,

Can you confirm you're not already working on this? Tony told me on
IRC you were doing something. I was just about to commit
ext/standard/var.c & tests. David and I are going to go through
ext/standard/*

Thanks
Olivier

Yep, after speaking with felipe, we (you and I) have standard/

17 years ago by Stanislav Malyshev — view source

unread

Hi!

Can you confirm you're not already working on this? Tony told me on
IRC you were doing something. I was just about to commit

Yes, I'm not working on ext/standard. I was doing Zend engine, but that
is done.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Alexey Zakhlestin — view source

unread

I did curl for 5.3

Hi!

While we nearing the release of 5.3 (hopefully?), there are many functions
in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

I have cleaned up Zend engine functions, converting them to the new API,
but there are about 1000 instances throughout PHP code (especially
ext/standard) which still use the old way. This way is less correct,
inconsistent with the new code, gives worse error messages, more prone to
various bugs, etc. All new extensions and functions use the new way, and the
only reason for keeping the old way in the code seems to be that nobody
cleaned it up. Somebody has to bring the old ones into sync, and I don't
think I personally will have time to do it in near timeframe. So if anybody
could step up to that - by doing at least part of the work - that'd be
great. The work is pretty simple, taking something like:

zval **data;

if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &data) == FAILURE) {
WRONG_PARAM_COUNT;
}
convert_to_string_ex(data);

and move it into:

char *str;
int str_len;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s",
&str, &str_len) == FAILURE) {
return;
}

and of course convert use of data zval into use of str.

This needs to be done carefully as not to break things on some more tricky
functions which may get arguments of multiple types, etc. Also, some tests
which check error messages may need to be fixed since new API gives more
detailed messages.
zend_parse_parameters documented here:
http://www.php.net/manual/en/internals2.ze1.zendapi.php
though some of the newer parameters aren't (C, h, f, Z) so one may need to
look at the code at zend_API.c.

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

--

--
Alexey Zakhlestin
http://blog.milkfarmsoft.com/

17 years ago by David Coallier — view source

unread

2008/6/21 Alexey Zakhlestin indeyets@gmail.com:

I did curl for 5.3

Grand! did you commit it already?

--
Slan,
David

17 years ago by Alexey Zakhlestin — view source

unread

2008/6/21 Alexey Zakhlestin indeyets@gmail.com:

I did curl for 5.3

I don't have karma.
did attachment come to the list?

anyway, here is the file: http://filez.indeyets.pp.ru/curl.diff

--
Alexey Zakhlestin
http://blog.milkfarmsoft.com/

17 years ago by Rasmus Lerdorf — view source

unread

Alexey Zakhlestin wrote:

2008/6/21 Alexey Zakhlestin indeyets@gmail.com:

I did curl for 5.3

I don't have karma.

You do now.

17 years ago by David Coallier — view source

unread

2008/6/21 Rasmus Lerdorf rasmus@lerdorf.com:

Alexey Zakhlestin wrote:

2008/6/21 Alexey Zakhlestin indeyets@gmail.com:

I did curl for 5.3

I don't have karma.

You do now.

Cheers Mr. L

--
Slan,
David

17 years ago by Alexey Zakhlestin — view source

unread

You do now.

thanks.
checked in

--
Alexey Zakhlestin
http://blog.milkfarmsoft.com/

17 years ago by Lukas Kahwe Smith — view source

unread

While we nearing the release of 5.3 (hopefully?), there are many
functions in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one
(zend_parse_parameters).

is this in any way related to the parameter parsing API change that
caused the BC break in array_merge() back in PHP 5.0? If so then I
would appreciate extra care to be taken to minimize these kinds of BC
breaks and at least make sure that these breaks are well documented.

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

17 years ago by David Coallier — view source

unread

2008/6/22 Lukas Kahwe Smith mls@pooteeweet.org:

While we nearing the release of 5.3 (hopefully?), there are many functions
in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

is this in any way related to the parameter parsing API change that caused
the BC break in array_merge() back in PHP 5.0? If so then I would appreciate
extra care to be taken to minimize these kinds of BC breaks and at least
make sure that these breaks are well documented.

Ok, with the tests being ran we are minimizing the chances of bc break
but I agree and extra care should be applied. We'll make sure this
care is applied.

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

--

--
Slan,
David

17 years ago by David Coallier — view source

unread

Ok, with the tests being ran we are minimizing the chances of bc break
but I agree and extra care should be applied. We'll make sure this
care is applied.

And sorry about the email before, I forgot to remove the junk. :)

--
Slan,
David

17 years ago by Pierre Joye — view source

unread

Hi David,

is this in any way related to the parameter parsing API change that caused
the BC break in array_merge() back in PHP 5.0? If so then I would appreciate
extra care to be taken to minimize these kinds of BC breaks and at least
make sure that these breaks are well documented.

Ok, with the tests being ran we are minimizing the chances of bc break
but I agree and extra care should be applied. We'll make sure this
care is applied.

http://wiki.php.net/doc/scratchpad/upgrade/53

--
Pierre
http://blog.thepimp.net | http://www.libgd.org

17 years ago by Olivier Hill — view source

unread

Ok, with the tests being ran we are minimizing the chances of bc break
but I agree and extra care should be applied. We'll make sure this
care is applied.

http://wiki.php.net/doc/scratchpad/upgrade/53

I will be documenting BC breaks on that page after committing.

Basically, some functions like trim() would not behave the same way
when passing things like resources, arrays or objects without a
__toString() method. The old ways didn't make any sense most of the
time anyway, as you don't want to trim a resource and get a string
back containing: "Resource"

Ex:

$ar = array(1, 2);

Before: var_dump(trim($ar)) -> string(5) "Array" with an E_NOTICE
Now: var_dump(trim($ar)) -> NULL with an E_WARNING

Tests are going to be changed to reflect that also (that's mostly the
longest part)

Regards,
Olivier

17 years ago by Stanislav Malyshev — view source

unread

Hi!

is this in any way related to the parameter parsing API change that
caused the BC break in array_merge() back in PHP 5.0? If so then I would

I don't really remember about array_merge in detail, but this change
shouldn't (I know, famous last words :) create any BC break unless you
rely on weird stuff like array-to-string implicit conversion (which you
really really shouldn't touch with a ten foot pole anyway).

Unfortunately, here we in general can do only as good as unit tests do,
so if anyone knows places where it might hurt, you may want to pay extra
attention there.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Lukas Kahwe Smith — view source

unread

Hi!

is this in any way related to the parameter parsing API change that
caused the BC break in array_merge() back in PHP 5.0? If so then I
would

I don't really remember about array_merge in detail, but this change
shouldn't (I know, famous last words :) create any BC break unless
you rely on weird stuff like array-to-string implicit conversion
(which you really really shouldn't touch with a ten foot pole anyway).

The break in array_merge() was that previously it would cast the
parameters to an array. As such a popular use case before the
parameter parsing change was the following:

$foo = null;
$bar = array_merge($foo, $array);

Since 5.0 this would lead to $bar being false and not $array. While it
could be argued that the user should do an explicit cast himself, it
was still a clear BC issue that caused a lot of issues for developers.

regards,
Lukas Kahwe Smith
mls@pooteeweet.org

17 years ago by Stanislav Malyshev — view source

unread

Hi!

Since 5.0 this would lead to $bar being false and not $array. While it
could be argued that the user should do an explicit cast himself, it was
still a clear BC issue that caused a lot of issues for developers.

Generally I think it would be nice if all functions behaved uniformly
(i.e. zend_parse_parameters has a standard, all use it). But we could
create special cases by accepting things as "z" and doing whatever we
want, I guess.

Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Timm Friebe — view source

unread

Hi,

While we nearing the release of 5.3 (hopefully?), there are many
functions in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

I started taking care of ext/sybase_ct.

Timm

17 years ago by crrodriguez@suse.de — view source

unread

Stanislav Malyshev escribió:

Hi!

While we nearing the release of 5.3 (hopefully?), there are many
functions in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

The attached patch may also help both core and PECL extensions, emiting
a deprecation compile warning when those functions are used.

ps: you have to add the attribute to all deprecated functions ;-)

--

Cristian Rodríguez R.
Platform/OpenSUSE - Core Services
SUSE LINUX Products GmbH
Research & Development
http://www.opensuse.org/

17 years ago by Stanislav Malyshev — view source

unread

Hi!

The attached patch may also help both core and PECL extensions, emiting
a deprecation compile warning when those functions are used.

I think it's a good idea. Any objections from anybody?

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com

17 years ago by Pierre Joye — view source

unread

Hi Christian,

Stanislav Malyshev escribió:

Hi!

While we nearing the release of 5.3 (hopefully?), there are many functions
in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one (zend_parse_parameters).

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

The attached patch may also help both core and PECL extensions, emiting a
deprecation compile warning when those functions are used.

That's a very good idea.

ps: you have to add the attribute to all deprecated functions ;-)

even better, a review while being at it cannot hurt :-)

Thanks for your patch!

--
Pierre

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

17 years ago by David Coallier — view source

unread

The attached patch may also help both core and PECL extensions, emiting a
deprecation compile warning when those functions are used.

A bit late to answer but very good idea :)

--
Slan,
David

17 years ago by Jordan Wambaugh — view source

unread

While we nearing the release of 5.3 (hopefully?), there are many
functions in the PHP code which still use old parameter parsing API
(zend_get_parameters_ex) instead of the new one
(zend_parse_parameters).

This is my first patch submission, so please let me know if you have
any pointers. I went ahead and took care of basic_functions.c (because
i figured I'd start with basics :-) ). I'll try to do some more
tonight. Please let me know if there are any areas you would like me
to focus on.

Cheers!

--
Jordan CM Wambaugh
jordan@wambaugh.org

Index: basic_functions.c

RCS file: /repository/php-src/ext/standard/basic_functions.c,v
retrieving revision 1.725.2.31.2.64.2.35
diff -u -r1.725.2.31.2.64.2.35 basic_functions.c
--- basic_functions.c 25 May 2008 14:06:13 -0000 1.725.2.31.2.64.2.35
+++ basic_functions.c 28 Jun 2008 01:56:16 -0000
@@ -4924,18 +4924,17 @@
/* }}} */

/* {{{ proto bool set_magic_quotes_runtime(int new_setting)

Set the current active configuration setting of
magic_quotes_runtime and return previous */

Set the current active configuration setting of
magic_quotes_runtime. Return true on success, false on failure.*/
PHP_FUNCTION(set_magic_quotes_runtime)
{

zval **new_setting;

zend_bool new_setting;

if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &new_setting)
== FAILURE) {

if (ZEND_NUM_ARGS() != 1 || zend_parse_parameters( 1 TSRMLS_CC, "b",
&new_setting ) == FAILURE) {
RETURN_FALSE;
}

convert_to_boolean_ex(new_setting);
PG(magic_quotes_runtime) = (zend_bool) Z_LVAL_PP(new_setting);

PG(magic_quotes_runtime) = new_setting;
RETURN_TRUE;
}
/* }}} */
@@ -4973,68 +4972,40 @@
Send an error message somewhere */
PHP_FUNCTION(error_log)
{

zval **string, **erropt = NULL, **option = NULL, **emailhead = NULL;

int opt_err = 0;

char *message, *opt = NULL, *headers = NULL;
switch (ZEND_NUM_ARGS()) {
```
  case 1:
```

  	if (zend_get_parameters_ex(1, &string) == FAILURE) {

  		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Argument 1 invalid");

```
  		RETURN_FALSE;
```
```
  	}
```
```
  	break;
```
```
  case 2:
```

  	if (zend_get_parameters_ex(2, &string, &erropt) == FAILURE) {

  		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid arguments");

```
  		RETURN_FALSE;
```
```
  	}
```
```
  	convert_to_long_ex(erropt);
```
```
  	opt_err = Z_LVAL_PP(erropt);
```
```
  	break;
```
```
  case 3:
```

  	if (zend_get_parameters_ex(3, &string, &erropt, &option) ==

FAILURE) {

  		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid arguments");

```
  		RETURN_FALSE;
```
```
  	}
```
```
  	convert_to_long_ex(erropt);
```
```
  	opt_err = Z_LVAL_PP(erropt);
```
```
  	convert_to_string_ex(option);
```
```
  	opt = Z_STRVAL_PP(option);
```
```
  	break;
```

char *message, *message_dup, *opt = NULL, *opt_dup, *headers = NULL,
*headers_dup;
int message_len, opt_len=0, headers_len=0;

```
  case 4:
```

  	if (zend_get_parameters_ex (4, &string, &erropt, &option,

&emailhead) == FAILURE) {

  		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid arguments");

```
  		RETURN_FALSE;
```
```
  	}
```
```
  	break;
```
```
  default:
```
```
  	WRONG_PARAM_COUNT;
```

if(ZEND_NUM_ARGS() < 1 || ZEND_NUM_ARGS() > 4){
```
  WRONG_PARAM_COUNT;
```
}

convert_to_string_ex(string);
message = Z_STRVAL_PP(string);
if (erropt != NULL) {
```
  convert_to_long_ex(erropt);
```
```
  opt_err = Z_LVAL_PP(erropt);
```

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|lss"
```
  						, &message, &message_len
```
```
  						, &opt_err
```
```
  						, &opt, &opt_len
```

  						, &headers, &headers_len) == FAILURE) {

  php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid Arguments");

```
  RETURN_FALSE;
```
}

if (option != NULL) {
```
  convert_to_string_ex(option);
```
```
  opt = Z_STRVAL_PP(option);
```
}

/convert our binary-safe strings to unsafe because that's what
_php_error_log() expects/
message_dup=estrndup(message, message_len);
message_dup=erealloc(message_dup,message_len+1);
message_dup[message_len]=0;
opt_dup=estrndup(opt, opt_len);
opt_dup=erealloc(opt_dup,opt_len+1);
opt_dup[opt_len]=0;
headers_dup=estrndup(headers, headers_len);
headers_dup=erealloc(headers_dup,headers_len+1);
headers_dup[headers_len]=0;

if (emailhead != NULL) {
```
  convert_to_string_ex(emailhead);
```
```
  headers = Z_STRVAL_PP(emailhead);
```
}
if (_php_error_log(opt_err, message, opt, headers TSRMLS_CC) ==
FAILURE) {

if (_php_error_log(opt_err, message_dup, opt_dup, headers_dup
TSRMLS_CC) == FAILURE) {
RETURN_FALSE;
}

17 years ago by Jordan Wambaugh — view source

unread

I have cleaned up Zend engine functions, converting them to the new
API, but there are about 1000 instances throughout PHP code
(especially ext/standard) which still use the old way. This way is
less correct, inconsistent with the new code, gives worse error
messages, more prone to various bugs, etc. All new extensions and
functions use the new way, and the only reason for keeping the old
way in the code seems to be that nobody cleaned it up. Somebody has
to bring the old ones into sync, and I don't think I personally will
have time to do it in near timeframe. So if anybody could step up to
that - by doing at least part of the work - that'd be great. The
work is pretty simple, taking something like:

I went ahead and took care of standard/assert.c and standard/
formatted_print.c for 5.3 as well. All tests are passing.

Cheers!

--
Jordan CM Wambaugh
jordan@wambaugh.org

Index: assert.c

RCS file: /repository/php-src/ext/standard/assert.c,v
retrieving revision 1.60.2.3.2.6.2.2
diff -u -r1.60.2.3.2.6.2.2 assert.c
--- assert.c 31 Dec 2007 07:17:14 -0000 1.60.2.3.2.6.2.2
+++ assert.c 28 Jun 2008 03:39:28 -0000
@@ -139,7 +139,7 @@
Checks if assertion is false */
PHP_FUNCTION(assert)
{

zval **assertion;

zval *assertion;
int val;
char *myeval = NULL;
char *compiled_string_description;
@@ -148,15 +148,15 @@
RETURN_TRUE;
}

if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &assertion) ==
FAILURE) {

if (ZEND_NUM_ARGS() != 1 || zend_parse_parameters(1 TSRMLS_CC, "z",
&assertion) == FAILURE) {
WRONG_PARAM_COUNT;
}

if (Z_TYPE_PP(assertion) == IS_STRING) {

if (Z_TYPE_PP(&assertion) == IS_STRING) {
zval retval;
int old_error_reporting = 0; /* shut up gcc! */

```
  myeval = Z_STRVAL_PP(assertion);
```

  myeval = Z_STRVAL_PP(&assertion);

  if (ASSERTG(quiet_eval)) {
  	old_error_reporting = EG(error_reporting);

@@ -181,8 +181,8 @@
convert_to_boolean(&retval);
val = Z_LVAL(retval);
} else {

```
  convert_to_boolean_ex(assertion);
```
```
  val = Z_LVAL_PP(assertion);
```

```
  convert_to_boolean_ex(&assertion);
```
```
  val = Z_LVAL_PP(&assertion);
```
}

if (val) {
@@ -235,26 +235,25 @@
}
/* }}} */

-/* {{{ proto mixed assert_options(int what [, mixed value])
+/* {{{ proto mixed assert_options(int what[, mixed value])
Set/get the various assert flags */
PHP_FUNCTION(assert_options)
{

zval **what, **value;
int oldint;

zval *value=0;
int oldint, what;
int ac = ZEND_NUM_ARGS();

if (ac < 1 || ac > 2 || zend_get_parameters_ex(ac, &what, &value) ==
FAILURE) {

if (ac <1 || ac > 2 || zend_parse_parameters(ac TSRMLS_CC, "i|z",
&what, &value) == FAILURE) {
WRONG_PARAM_COUNT;
}

convert_to_long_ex(what);
switch (Z_LVAL_PP(what)) {

switch (what) {
case ASSERT_ACTIVE:
oldint = ASSERTG(active);
if (ac == 2) {

```
  	convert_to_long_ex(value);
```
```
  	ASSERTG(active) = Z_LVAL_PP(value);
```

```
  	convert_to_long_ex(&value);
```

  	ASSERTG(active) = Z_LVAL_PP(&value);
  }
  RETURN_LONG(oldint);
  break;

@@ -262,8 +261,8 @@
case ASSERT_BAIL:
oldint = ASSERTG(bail);
if (ac == 2) {

```
  	convert_to_long_ex(value);
```
```
  	ASSERTG(bail) = Z_LVAL_PP(value);
```

```
  	convert_to_long_ex(&value);
```

  	ASSERTG(bail) = Z_LVAL_PP(&value);
  }
  RETURN_LONG(oldint);
  break;

@@ -271,8 +270,8 @@
case ASSERT_QUIET_EVAL:
oldint = ASSERTG(quiet_eval);
if (ac == 2) {

```
  	convert_to_long_ex(value);
```

  	ASSERTG(quiet_eval) = Z_LVAL_PP(value);

```
  	convert_to_long_ex(&value);
```

  	ASSERTG(quiet_eval) = Z_LVAL_PP(&value);
  }
  RETURN_LONG(oldint);
  break;

@@ -280,8 +279,8 @@
case ASSERT_WARNING:
oldint = ASSERTG(warning);
if (ac == 2) {

```
  	convert_to_long_ex(value);
```

  	ASSERTG(warning) = Z_LVAL_PP(value);

```
  	convert_to_long_ex(&value);
```

  	ASSERTG(warning) = Z_LVAL_PP(&value);
  }
  RETURN_LONG(oldint);
  break;

@@ -298,14 +297,14 @@
if (ASSERTG(callback)) {
zval_ptr_dtor(&ASSERTG(callback));
}

```
  	ASSERTG(callback) = *value;
```
```
  	zval_add_ref(value);
```

```
  	ASSERTG(callback) = value;
```

  	zval_add_ref(&value);
  }
  return;
  break;

default:

  php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown value %ld",

Z_LVAL_PP(what));

  php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown value %ld",

what);
break;
}

Index: formatted_print.c

RCS file: /repository/php-src/ext/standard/formatted_print.c,v
retrieving revision 1.82.2.1.2.16.2.6
diff -u -r1.82.2.1.2.16.2.6 formatted_print.c
--- formatted_print.c 25 Jun 2008 10:16:52 -0000 1.82.2.1.2.16.2.6
+++ formatted_print.c 28 Jun 2008 03:34:12 -0000
@@ -731,19 +731,19 @@
PHP_FUNCTION(fprintf)
{
php_stream *stream;

zval **arg1;

zval *arg1;
char *result;
int len;

if (ZEND_NUM_ARGS() < 2) {
WRONG_PARAM_COUNT;
}

if (zend_get_parameters_ex(1, &arg1)==FAILURE) {

if (zend_parse_parameters(1 TSRMLS_CC, "z", &arg1) ==FAILURE){
RETURN_FALSE;
}

php_stream_from_zval(stream, arg1);

php_stream_from_zval(stream, &arg1);

if ((result=php_formatted_print(ht, &len, 0, 1 TSRMLS_CC))==NULL) {
RETURN_FALSE;
@@ -762,19 +762,20 @@
PHP_FUNCTION(vfprintf)
{
php_stream *stream;

zval **arg1;

zval *arg1;
char *result;
int len;

if (ZEND_NUM_ARGS() != 3) {
WRONG_PARAM_COUNT;
}

if (zend_get_parameters_ex(1, &arg1)==FAILURE) {

if (zend_parse_parameters(1 TSRMLS_CC, "z", &arg1) ==FAILURE){
RETURN_FALSE;
}

php_stream_from_zval(stream, arg1);

php_stream_from_zval(stream, &arg1);

if ((result=php_formatted_print(ht, &len, 1, 1 TSRMLS_CC))==NULL) {
RETURN_FALSE;

17 years ago by David Coallier — view source

unread

2008/6/28 Jordan Wambaugh jordan@wambaugh.org:

I have cleaned up Zend engine functions, converting them to the new API,
but there are about 1000 instances throughout PHP code (especially
ext/standard) which still use the old way. This way is less correct,
inconsistent with the new code, gives worse error messages, more prone to
various bugs, etc. All new extensions and functions use the new way, and the
only reason for keeping the old way in the code seems to be that nobody
cleaned it up. Somebody has to bring the old ones into sync, and I don't
think I personally will have time to do it in near timeframe. So if anybody
could step up to that - by doing at least part of the work - that'd be
great. The work is pretty simple, taking something like:

I went ahead and took care of standard/assert.c and
standard/formatted_print.c for 5.3 as well. All tests are passing.

Cheers!

--
Jordan CM Wambaugh
jordan@wambaugh.org

Index: assert.c

RCS file: /repository/php-src/ext/standard/assert.c,v
retrieving revision 1.60.2.3.2.6.2.2
diff -u -r1.60.2.3.2.6.2.2 assert.c
--- assert.c 31 Dec 2007 07:17:14 -0000 1.60.2.3.2.6.2.2
+++ assert.c 28 Jun 2008 03:39:28 -0000
@@ -139,7 +139,7 @@
Checks if assertion is false */
PHP_FUNCTION(assert)
{
  zval **assertion;
  zval *assertion;
 int val;
 char *myeval = NULL;
 char *compiled_string_description;
@@ -148,15 +148,15 @@
RETURN_TRUE;
}
  if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &assertion) ==
FAILURE) {
  if (ZEND_NUM_ARGS() != 1 || zend_parse_parameters(1 TSRMLS_CC, "z",
&assertion) == FAILURE) {
WRONG_PARAM_COUNT;
}
  if (Z_TYPE_PP(assertion) == IS_STRING) {
  if (Z_TYPE_PP(&assertion) == IS_STRING) {
         zval retval;
         int old_error_reporting = 0; /* shut up gcc! */
          myeval = Z_STRVAL_PP(assertion);
          myeval = Z_STRVAL_PP(&assertion);

         if (ASSERTG(quiet_eval)) {
                 old_error_reporting = EG(error_reporting);
@@ -181,8 +181,8 @@
convert_to_boolean(&retval);
val = Z_LVAL(retval);
} else {
          convert_to_boolean_ex(assertion);
          val = Z_LVAL_PP(assertion);
          convert_to_boolean_ex(&assertion);
          val = Z_LVAL_PP(&assertion);
 }

 if (val) {
@@ -235,26 +235,25 @@
}
/* }}} */

-/* {{{ proto mixed assert_options(int what [, mixed value])
+/* {{{ proto mixed assert_options(int what[, mixed value])
Set/get the various assert flags */
PHP_FUNCTION(assert_options)
{
  zval **what, **value;
  int oldint;
  zval *value=0;
  int oldint, what;
 int ac = ZEND_NUM_ARGS();
  if (ac < 1 || ac > 2 || zend_get_parameters_ex(ac, &what, &value) ==
FAILURE) {
  if (ac <1 || ac > 2 || zend_parse_parameters(ac TSRMLS_CC, "i|z",
&what, &value) == FAILURE) {
WRONG_PARAM_COUNT;
}
  convert_to_long_ex(what);
  switch (Z_LVAL_PP(what)) {
  switch (what) {
 case ASSERT_ACTIVE:
         oldint = ASSERTG(active);
         if (ac == 2) {
                  convert_to_long_ex(value);
                  ASSERTG(active) = Z_LVAL_PP(value);
                  convert_to_long_ex(&value);
                  ASSERTG(active) = Z_LVAL_PP(&value);
         }
         RETURN_LONG(oldint);
         break;
@@ -262,8 +261,8 @@
case ASSERT_BAIL:
oldint = ASSERTG(bail);
if (ac == 2) {
                  convert_to_long_ex(value);
                  ASSERTG(bail) = Z_LVAL_PP(value);
                  convert_to_long_ex(&value);
                  ASSERTG(bail) = Z_LVAL_PP(&value);
         }
         RETURN_LONG(oldint);
         break;
@@ -271,8 +270,8 @@
case ASSERT_QUIET_EVAL:
oldint = ASSERTG(quiet_eval);
if (ac == 2) {
                  convert_to_long_ex(value);
                  ASSERTG(quiet_eval) = Z_LVAL_PP(value);
                  convert_to_long_ex(&value);
                  ASSERTG(quiet_eval) = Z_LVAL_PP(&value);
         }
         RETURN_LONG(oldint);
         break;
@@ -280,8 +279,8 @@
case ASSERT_WARNING:
oldint = ASSERTG(warning);
if (ac == 2) {
                  convert_to_long_ex(value);
                  ASSERTG(warning) = Z_LVAL_PP(value);
                  convert_to_long_ex(&value);
                  ASSERTG(warning) = Z_LVAL_PP(&value);
         }
         RETURN_LONG(oldint);
         break;
@@ -298,14 +297,14 @@
if (ASSERTG(callback)) {
zval_ptr_dtor(&ASSERTG(callback));
}
                  ASSERTG(callback) = *value;
                  zval_add_ref(value);
                  ASSERTG(callback) = value;
                  zval_add_ref(&value);
         }
         return;
         break;

 default:
          php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown value
%ld", Z_LVAL_PP(what));
          php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown value
%ld", what);
break;
}

Index: formatted_print.c

RCS file: /repository/php-src/ext/standard/formatted_print.c,v
retrieving revision 1.82.2.1.2.16.2.6
diff -u -r1.82.2.1.2.16.2.6 formatted_print.c
--- formatted_print.c 25 Jun 2008 10:16:52 -0000 1.82.2.1.2.16.2.6
+++ formatted_print.c 28 Jun 2008 03:34:12 -0000
@@ -731,19 +731,19 @@
PHP_FUNCTION(fprintf)
{
php_stream *stream;
  zval **arg1;
  zval *arg1;
 char *result;
 int len;

 if (ZEND_NUM_ARGS() < 2) {
         WRONG_PARAM_COUNT;
 }
  if (zend_get_parameters_ex(1, &arg1)==FAILURE) {
  if (zend_parse_parameters(1 TSRMLS_CC, "z", &arg1) ==FAILURE){
         RETURN_FALSE;
 }
  php_stream_from_zval(stream, arg1);
  php_stream_from_zval(stream, &arg1);

 if ((result=php_formatted_print(ht, &len, 0, 1 TSRMLS_CC))==NULL) {
         RETURN_FALSE;
@@ -762,19 +762,20 @@
PHP_FUNCTION(vfprintf)
{
php_stream *stream;
  zval **arg1;
  zval *arg1;
 char *result;
 int len;

 if (ZEND_NUM_ARGS() != 3) {
         WRONG_PARAM_COUNT;
 }
  if (zend_get_parameters_ex(1, &arg1)==FAILURE) {
  if (zend_parse_parameters(1 TSRMLS_CC, "z", &arg1) ==FAILURE){
         RETURN_FALSE;
 }
  php_stream_from_zval(stream, arg1);
  php_stream_from_zval(stream, &arg1);

 if ((result=php_formatted_print(ht, &len, 1, 1 TSRMLS_CC))==NULL) {
         RETURN_FALSE;

The idea of the new parsing parameter is to catch the number of
parameters as well. For instance when you have

   if (ZEND_NUM_ARGS() < 2) {
           WRONG_PARAM_COUNT;
   }

then your zend parse param function call should have 2 params or more
Ex:

int argc;
char *name;
zval **other;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sZ", &name,

&other, &argc) == FAILURE) {
return;
}

This way of parsing the parameters remove the ZEND_NUM_ARGS() and
WRONG_PARAM_COUNT and adds consistency in the return error messages
across the language.

Also next time please read the previous entries of the thread :

See: http://url.ie/hck that was about the 3rd or 4th message.

Anyways that's ok, I lot of conflicts now in formatted_print.c and
assert.c but I'll work around them to get the resolved. We just have
to get organized on which files we'll be working on then. Right now,
Olivier is working on file.c and has done string.c so please don't do
those 2 files.

--
Slan,
David

17 years ago by Jordan Wambaugh — view source

unread

The idea of the new parsing parameter is to catch the number of
parameters as well. For instance when you have
  if (ZEND_NUM_ARGS() < 2) {
          WRONG_PARAM_COUNT;
  }
then your zend parse param function call should have 2 params or more
Ex:

int argc;
char *name;
zval **other;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sZ", &name,
&other, &argc) == FAILURE) {
return;
}

This way of parsing the parameters remove the ZEND_NUM_ARGS() and
WRONG_PARAM_COUNT and adds consistency in the return error messages
across the language.

Also next time please read the previous entries of the thread :

See: http://url.ie/hck that was about the 3rd or 4th message.

Anyways that's ok, I lot of conflicts now in formatted_print.c and
assert.c but I'll work around them to get the resolved. We just have
to get organized on which files we'll be working on then. Right now,
Olivier is working on file.c and has done string.c so please don't do
those 2 files.

Ok, sorry, I didn't catch the thread where you said you were going to
work on standard. Do you want me to fix it so I'm not checking
ZEND_NUM_ARGS, or should I just leave that to you?
I noticed mysql needs some love... anyone working on mysql, or can I
take it?

--
Jordan CM Wambaugh
jordan@wambaugh.org

17 years ago by David Coallier — view source

unread

2008/6/28 Jordan Wambaugh jordan@wambaugh.org:

The idea of the new parsing parameter is to catch the number of
parameters as well. For instance when you have
 if (ZEND_NUM_ARGS() < 2) {
         WRONG_PARAM_COUNT;
 }
then your zend parse param function call should have 2 params or more
Ex:

int argc;
char *name;
zval **other;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sZ", &name,
&other, &argc) == FAILURE) {
return;
}

This way of parsing the parameters remove the ZEND_NUM_ARGS() and
WRONG_PARAM_COUNT and adds consistency in the return error messages
across the language.

Also next time please read the previous entries of the thread :

See: http://url.ie/hck that was about the 3rd or 4th message.

Anyways that's ok, I lot of conflicts now in formatted_print.c and
assert.c but I'll work around them to get the resolved. We just have
to get organized on which files we'll be working on then. Right now,
Olivier is working on file.c and has done string.c so please don't do
those 2 files.
Ok, sorry, I didn't catch the thread where you said you were going to work
on standard. Do you want me to fix it so I'm not checking ZEND_NUM_ARGS, or
should I just leave that to you?

Go ahead, you are pretty well started, I'll adjust after you are done :)

--
Slan,
David

17 years ago by Jordan Wambaugh — view source

unread

2008/6/28 Jordan Wambaugh jordan@wambaugh.org:
The idea of the new parsing parameter is to catch the number of
parameters as well. For instance when you have
if (ZEND_NUM_ARGS() < 2) {
        WRONG_PARAM_COUNT;
}
then your zend parse param function call should have 2 params or
more
Ex:

int argc;
char *name;
zval **other;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sZ", &name,
&other, &argc) == FAILURE) {
return;
}

This way of parsing the parameters remove the ZEND_NUM_ARGS() and
WRONG_PARAM_COUNT and adds consistency in the return error messages
across the language.

Also next time please read the previous entries of the thread :

See: http://url.ie/hck that was about the 3rd or 4th message.

Anyways that's ok, I lot of conflicts now in formatted_print.c and
assert.c but I'll work around them to get the resolved. We just have
to get organized on which files we'll be working on then. Right now,
Olivier is working on file.c and has done string.c so please don't
do
those 2 files.
Ok, sorry, I didn't catch the thread where you said you were going
to work
on standard. Do you want me to fix it so I'm not checking
ZEND_NUM_ARGS, or
should I just leave that to you?
Go ahead, you are pretty well started, I'll adjust after you are
done :)

Alright, I rewrote my patches for basic_functions.c, assert.c, and
formatted_print.c (attached) a few notes:

fprintf and vsprintf - I'm not sure if this is the appropriate way to
work with a variable number of parameters, please let me know if there
is a better way. Also, because zend_parse_parameters is generating a
different error message if you don't pass enough parameters, the
*_error tests are failing. I didn't want to change any of the tests
until my patch has been reviewed, in case changes need to be made.
Also, set_magic_quotes_runtime is not issuing a deprecated warning,
but the test is expecting it, causing some tests to fail. I noticed in
the documentation (http://us3.php.net/manual/en/function.set-magic-quotes-runtime.php
) that its not documented as being deprecated, so I'm not sure what
the expectation should be in the test.

17 years ago by Jordan Wambaugh — view source

unread

2008/6/28 Jordan Wambaugh jordan@wambaugh.org:

The idea of the new parsing parameter is to catch the number of
parameters as well. For instance when you have

if (ZEND_NUM_ARGS() < 2) {
WRONG_PARAM_COUNT;
}

then your zend parse param function call should have 2 params or
more
Ex:

int argc;
char *name;
zval **other;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sZ", &name,
&other, &argc) == FAILURE) {
return;
}

This way of parsing the parameters remove the ZEND_NUM_ARGS() and
WRONG_PARAM_COUNT and adds consistency in the return error messages
across the language.

Also next time please read the previous entries of the thread :

See: http://url.ie/hck that was about the 3rd or 4th message.

Anyways that's ok, I lot of conflicts now in formatted_print.c and
assert.c but I'll work around them to get the resolved. We just
have
to get organized on which files we'll be working on then. Right
now,
Olivier is working on file.c and has done string.c so please
don't do
those 2 files.

Ok, sorry, I didn't catch the thread where you said you were going
to work
on standard. Do you want me to fix it so I'm not checking
ZEND_NUM_ARGS, or
should I just leave that to you?

Go ahead, you are pretty well started, I'll adjust after you are
done :)

Alright, I rewrote my patches for basic_functions.c, assert.c, and
formatted_print.c (attached) a few notes:

fprintf and vsprintf - I'm not sure if this is the appropriate way
to work with a variable number of parameters, please let me know if
there is a better way. Also, because zend_parse_parameters is
generating a different error message if you don't pass enough
parameters, the *_error tests are failing. I didn't want to change
any of the tests until my patch has been reviewed, in case changes
need to be made.
Also, set_magic_quotes_runtime is not issuing a deprecated warning,
but the test is expecting it, causing some tests to fail. I noticed
in the documentation (http://us3.php.net/manual/en/function.set-magic-quotes-runtime.php
) that its not documented as being deprecated, so I'm not sure what
the expectation should be in the test.

I noticed a couple problems in my patch for error_log (a memory leak,
for example). Here's a new version of the complete patch with those
issues corrected.

--
Jordan CM Wambaugh

cleaning up the functions - any volunteers?

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

Yes, I'm not working on ext/standard. I was doing Zend engine, but that
is done.

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

Unfortunately, here we in general can do only as good as unit tests do,
so if anyone knows places where it might hurt, you may want to pay extra
attention there.

Generally I think it would be nice if all functions behaved uniformly
(i.e. zend_parse_parameters has a standard, all use it). But we could
create special cases by accepting things as "z" and doing whatever we
want, I guess.

Index: basic_functions.c

Index: assert.c

Index: formatted_print.c

Index: assert.c

Index: formatted_print.c

cleaning up the functions - any volunteers?

P.S. if some genius would invent a script to automate that - it'd be awesome, but I don't have very high hopes on that. :)

P.S. if some genius would invent a script to automate that - it'd be awesome, but I don't have very high hopes on that. :)

Yes, I'm not working on ext/standard. I was doing Zend engine, but that is done.

P.S. if some genius would invent a script to automate that - it'd be awesome, but I don't have very high hopes on that. :)

Unfortunately, here we in general can do only as good as unit tests do, so if anyone knows places where it might hurt, you may want to pay extra attention there.

Generally I think it would be nice if all functions behaved uniformly (i.e. zend_parse_parameters has a standard, all use it). But we could create special cases by accepting things as "z" and doing whatever we want, I guess.

Index: basic_functions.c

Index: assert.c

Index: formatted_print.c

Index: assert.c

Index: formatted_print.c

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

Yes, I'm not working on ext/standard. I was doing Zend engine, but that
is done.

P.S. if some genius would invent a script to automate that - it'd be
awesome, but I don't have very high hopes on that. :)

Unfortunately, here we in general can do only as good as unit tests do,
so if anyone knows places where it might hurt, you may want to pay extra
attention there.

Generally I think it would be nice if all functions behaved uniformly
(i.e. zend_parse_parameters has a standard, all use it). But we could
create special cases by accepting things as "z" and doing whatever we
want, I guess.