Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
Hello All, After reviewing Christian Dickmann's list of functions with broken prototypes, I have started making corrections where needed. Given that I am not sure that everyone will agree that these changes are needed (or that I will make the changes properly FTM), I am starting slowly. :) The attached patch corrects the proto, improves the proto description and makes the function return FALSE when it encounters an error. Feedback is welcome! I realize that someone more skilled could do this work about 20x faster. However, I would like to tackle small tasks like this to get more familiar with the source. :) Sure, commit it... But please don't send a patch for every proto change, we'll yell out you fine if you commit something bad... ;) Btw, yes, if you wouldn't mind zend_parse_parameters() conversion would be great, its probably a little trickier than protos, but, its what we want all the source to be using in X years/months/days/minutes/seconds. -Sterling -- Zak Greant PHP Quality Assurance Team http://qa.php.net/ We must be the change we wish to see. - M. K. Ghandi Content-Description: Unified diff for ext/standard/array.c -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED] -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On Wed, 12 Dec 2001, Sterling Hughes wrote: Sure, commit it... But please don't send a patch for every proto change, we'll yell out you fine if you commit something bad... ;) Btw, yes, if you wouldn't mind zend_parse_parameters() conversion would be great, its probably a little trickier than protos, but, its what we want all the source to be using in X years/months/days/minutes/seconds. Well, except for very performance-critical functions, I guess, since it is somewhat slower than pure zend_get_parameters_ex(). -Andrei Complexity in the mind is not caused by learning; learning is caused by complexity in the mind. -- Steven Pinker -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On Wed, 12 Dec 2001, Sterling Hughes wrote: Sure, commit it... But please don't send a patch for every proto change, we'll yell out you fine if you commit something bad... ;) Btw, yes, if you wouldn't mind zend_parse_parameters() conversion would be great, its probably a little trickier than protos, but, its what we want all the source to be using in X years/months/days/minutes/seconds. Well, except for very performance-critical functions, I guess, since it is somewhat slower than pure zend_get_parameters_ex(). And those functions would be? :) Are you talking about regularly used functions such as strlen(), etc.? I agree that zend_basic_functions.c should remain abstracted, I guess where we draw the line is a bit fuzzy to me? What are your thoughts as to what functions should remain un-zend-parse-parameterized (say *that* 10 times fast :). -Sterling -Andrei Complexity in the mind is not caused by learning; learning is caused by complexity in the mind. -- Steven Pinker -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On Wed, 12 Dec 2001, Sterling Hughes wrote: And those functions would be? :) Are you talking about regularly used functions such as strlen(), etc.? Functions like count(), for example, that people tend to use in a loop unnecessarily. I agree that zend_basic_functions.c should remain abstracted, I guess where we draw the line is a bit fuzzy to me? What are your thoughts as to what functions should remain un-zend-parse-parameterized (say *that* 10 times fast :). Yes, the distinction is not clear. Perhaps, the person who does the conversion for each module could do some benchmark tests on their programs and see what might need to be adjusted. -Andrei I still find each day too short for all the thoughts I want to think, all the walks I want to take, all the books I want to read, and all the friends I want to see. -John Burroughs -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On December 12, 2001 03:54 am, Sterling Hughes wrote: [...] Sure, commit it... But please don't send a patch for every proto change, we'll yell out you fine if you commit something bad... ;) C'mon - the list needs the extra traffic to appear active. :) To commit to the source code, I will need extra karma. Btw, yes, if you wouldn't mind zend_parse_parameters() conversion would be great, its probably a little trickier than protos, but, its what we want all the source to be using in X years/months/days/minutes/seconds. I will start doing this (taking into account Andrei's constraints) Also, is there a good source of documentation on zend_parse_parameters(), or should I run throught the dev messages and create some. --zak -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On Wed, 12 Dec 2001, Zak Greant wrote: I will start doing this (taking into account Andrei's constraints) Also, is there a good source of documentation on zend_parse_parameters(), or should I run throught the dev messages and create some. README.PARAMETER_PARSING_API -Andrei * The future is not what it used to be. * -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On Wed, Dec 12, 2001 at 12:44:21PM -0600, Andrei Zmievski wrote : On Wed, 12 Dec 2001, Zak Greant wrote: I will start doing this (taking into account Andrei's constraints) Also, is there a good source of documentation on zend_parse_parameters(), or should I run throught the dev messages and create some. README.PARAMETER_PARSING_API Or, tata, http://www.zend.com/apidoc/x837.php -- Please always Cc to me when replying to me on the lists. -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On December 12, 2001 07:33 am, Andrei Zmievski wrote:
On Wed, 12 Dec 2001, Sterling Hughes wrote:
And those functions would be? :) Are you talking about
regularly used functions such as strlen(), etc.?
Functions like count(), for example, that people tend to use in a
loop unnecessarily.
I agree that zend_basic_functions.c should remain abstracted, I
guess where we draw the line is a bit fuzzy to me? What are
your thoughts as to what functions should remain
un-zend-parse-parameterized (say *that* 10 times fast :).
Yes, the distinction is not clear. Perhaps, the person who does the
conversion for each module could do some benchmark tests on their
programs and see what might need to be adjusted.
I did a simple test with krsort before and after zend_parse_parameters.
See the attached files for information on the changes that I made and
the test that I used. In this case, it looks like we face at most about
a 0.0001 second penalty per function call for using
zend_parse_parameters. This is based on the data below: (4.762 seconds
- 3.704 seconds) / 1 calls to krsort)
Profiling would probably be a better choice, however, I am having a few
troubles getting profiling going with the PHP source. :)
As always, correction and comments welcome.
---
With zend_parse_parameters():
real0m4.762s
user0m4.710s
sys 0m0.030s
real0m4.738s
user0m4.700s
sys 0m0.030s
real0m4.726s
user0m4.700s
sys 0m0.020s
Without zend_parse_parameters():
real0m3.704s
user0m3.620s
sys 0m0.040s
real0m3.833s
user0m3.660s
sys 0m0.020s
real0m3.764s
user0m3.650s
sys 0m0.010s
--
Zak Greant
PHP Quality Assurance Team
http://qa.php.net/
We must be the change we wish to see. - M. K. Ghandi
?php
foreach (file ('/usr/local/src/data.txt') as $k = $v) {
$array[$v] = $k;
}
define (MAX, 1);
for ($x=0; $x MAX; ++$x) {
$data = $array;
krsort ($data);
}
?
Index: ext/standard/array.c
===
RCS file: /repository/php4/ext/standard/array.c,v
retrieving revision 1.148
diff -u -r1.148 array.c
--- ext/standard/array.c 11 Dec 2001 15:30:27 - 1.148
+++ ext/standard/array.c 13 Dec 2001 07:19:29 -
@@ -179,39 +179,34 @@
return array_key_compare(a, b TSRMLS_CC) * -1;
}
-/* {{{ proto int krsort(array array_arg [, int sort_flags])
- Sort an array reverse by key */
+/* {{{ proto bool krsort(array array_arg [, int sort_flags])
+ Sort an array by key value in reverse order */
PHP_FUNCTION(krsort)
{
- zval **array, **sort_type;
- int sort_type_val = SORT_REGULAR;
+ zval *array;
+ long sort_type_val = SORT_REGULAR;
HashTable *target_hash;
- if (ZEND_NUM_ARGS() 1 || ZEND_NUM_ARGS() 2 ||
- zend_get_parameters_ex(ZEND_NUM_ARGS(), array, sort_type) == FAILURE) {
- WRONG_PARAM_COUNT;
- }
- target_hash = HASH_OF(*array);
- if (!target_hash) {
- php_error(E_WARNING, Wrong datatype in krsort() call);
- return;
- }
- if (ZEND_NUM_ARGS() == 2) {
- convert_to_long_ex(sort_type);
- sort_type_val = Z_LVAL_PP(sort_type);
+ if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, a|l, array, sort_type_val) == FAILURE) {
+ RETURN_FALSE;
}
+
+ target_hash = HASH_OF(array);
set_compare_func(sort_type_val TSRMLS_CC);
+
if (zend_hash_sort(target_hash, zend_qsort, array_reverse_key_compare, 0 TSRMLS_CC) == FAILURE) {
- return;
+ RETURN_FALSE;
}
+
RETURN_TRUE;
}
/* }}} */
CODING_STANDARDS
CREDITS
CVS
ChangeLog
ChangeLog.1999.gz
ChangeLog.2000.gz
EXTENSIONS
INSTALL
LICENSE
Makefile
Makefile.in
NEWS
README.CVS-RULES
README.EXTENSIONS
README.EXT_SKEL
README.PARAMETER_PARSING_API
README.QNX
README.SELF-CONTAINED-EXTENSIONS
README.STREAMS
README.Zeus
RELEASE_PROCESS
TODO
TODO-4.2.txt
TSRM
Zend
acconfig.h
acconfig.h.in
acinclude.m4
aclocal.m4
apidoc-zend.txt
apidoc.txt
build
buildconf
buildmk.stamp
config.guess
config.log
config.nice
config.status
config.sub
config_vars.mk
configure
configure.in
cvsclean
dynlib.m4
ext
footer
generated_lists
genfiles
header
install-sh
libphp4.la
libs
libtool
ltmain.sh
main
makedist
makerpm
missing
mkinstalldirs
modules
pear
php.ini-dist
php.ini-recommended
php4.gif
php4.spec
php4.spec.in
php_version.h
regex
run-tests.php
sapi
scripts
snapshot
stamp-h.in
stub.c
stub.lo
stub.o
tests
win32
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
[PHP-DEV] [patch] krsort() ext/standard/array.c
Hello All,
After reviewing Christian Dickmann's list of functions with broken
prototypes, I have started making corrections where needed.
Given that I am not sure that everyone will agree that these changes
are needed (or that I will make the changes properly FTM), I am
starting slowly. :)
The attached patch corrects the proto, improves the proto description
and makes the function return FALSE when it encounters an error.
Feedback is welcome!
I realize that someone more skilled could do this work about 20x
faster. However, I would like to tackle small tasks like this to get
more familiar with the source. :)
--
Zak Greant
PHP Quality Assurance Team
http://qa.php.net/
We must be the change we wish to see. - M. K. Ghandi
Index: ext/standard/array.c
===
RCS file: /repository/php4/ext/standard/array.c,v
retrieving revision 1.148
diff -u -r1.148 array.c
--- ext/standard/array.c 11 Dec 2001 15:30:27 - 1.148
+++ ext/standard/array.c 12 Dec 2001 01:44:22 -
@@ -179,8 +179,8 @@
return array_key_compare(a, b TSRMLS_CC) * -1;
}
-/* {{{ proto int krsort(array array_arg [, int sort_flags])
- Sort an array reverse by key */
+/* {{{ proto bool krsort(array array_arg [, int sort_flags])
+ Sort an array by key value in reverse order */
PHP_FUNCTION(krsort)
{
zval **array, **sort_type;
@@ -194,7 +194,7 @@
target_hash = HASH_OF(*array);
if (!target_hash) {
php_error(E_WARNING, Wrong datatype in krsort() call);
- return;
+ RETURN_FALSE
}
if (ZEND_NUM_ARGS() == 2) {
convert_to_long_ex(sort_type);
@@ -202,7 +202,7 @@
}
set_compare_func(sort_type_val TSRMLS_CC);
if (zend_hash_sort(target_hash, zend_qsort, array_reverse_key_compare, 0 TSRMLS_CC) == FAILURE) {
- return;
+ RETURN_FALSE;
}
RETURN_TRUE;
}
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [patch] krsort() ext/standard/array.c
On Tue, Dec 11, 2001 at 10:12:02PM -0700, Zak Greant wrote : Also, when touching up functions, is there any significant advantage to migrating them to zend_parse_parameters? Automatic type conversion and less code and unified error messages. Less code btw. it is also cleaner and easier to read. You can easily spot what gets parsed/cast into which type. But like Andrei said once, it doesn't cure all ills hehe :) - Markus -- Please always Cc to me when replying to me on the lists. -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
