[PHP-CVS] com php-src: Refactor to using a stack based zval instead of dynamic allocation: ext/standard/password.c

[Anthony Ferrara]

Commit:0bc9ca39ced4128c3b9fb1ba2ac797d342e7eef2
Author:Anthony Ferrara ircmax...@gmail.com Sun, 7 Oct 2012 
05:42:08 -0400
Parents:   37b2207f66ac1cebdc3ff3f7f88ec319ee893292
Branches:  master

Link:   
http://git.php.net/?p=php-src.git;a=commitdiff;h=0bc9ca39ced4128c3b9fb1ba2ac797d342e7eef2

Log:
Refactor to using a stack based zval instead of dynamic allocation

Changed paths:
  M  ext/standard/password.c


Diff:
diff --git a/ext/standard/password.c b/ext/standard/password.c
index 3507183..266ad0a 100644
--- a/ext/standard/password.c
+++ b/ext/standard/password.c
@@ -245,12 +245,11 @@ PHP_FUNCTION(password_needs_rehash)

if (options  zend_symtable_find(options, 
cost, sizeof(cost), (void **) option_buffer) == SUCCESS) {
if (Z_TYPE_PP(option_buffer) != 
IS_LONG) {
-   zval *cast_option_buffer;
-   ALLOC_ZVAL(cast_option_buffer);
-   MAKE_COPY_ZVAL(option_buffer, 
cast_option_buffer);
-   
convert_to_long(cast_option_buffer);
-   new_cost = 
Z_LVAL_P(cast_option_buffer);
-   
zval_ptr_dtor(cast_option_buffer);
+   zval cast_option_buffer;
+   MAKE_COPY_ZVAL(option_buffer, 
cast_option_buffer);
+   
convert_to_long(cast_option_buffer);
+   new_cost = 
Z_LVAL(cast_option_buffer);
+   zval_dtor(cast_option_buffer);
} else {
new_cost = 
Z_LVAL_PP(option_buffer);
}
@@ -326,12 +325,11 @@ PHP_FUNCTION(password_hash)

if (options  zend_symtable_find(options, cost, 5, 
(void **) option_buffer) == SUCCESS) {
if (Z_TYPE_PP(option_buffer) != IS_LONG) {
-   zval *cast_option_buffer;
-   ALLOC_ZVAL(cast_option_buffer);
-   MAKE_COPY_ZVAL(option_buffer, 
cast_option_buffer);
-   convert_to_long(cast_option_buffer);
-   cost = Z_LVAL_P(cast_option_buffer);
-   zval_ptr_dtor(cast_option_buffer);
+   zval cast_option_buffer;
+   MAKE_COPY_ZVAL(option_buffer, 
cast_option_buffer);
+   convert_to_long(cast_option_buffer);
+   cost = Z_LVAL(cast_option_buffer);
+   zval_dtor(cast_option_buffer);
} else {
cost = Z_LVAL_PP(option_buffer);
}
@@ -366,17 +364,16 @@ PHP_FUNCTION(password_hash)
case IS_LONG:
case IS_DOUBLE:
case IS_OBJECT: {
-   zval *cast_option_buffer;
-   ALLOC_ZVAL(cast_option_buffer);
-   MAKE_COPY_ZVAL(option_buffer, 
cast_option_buffer);
-   convert_to_string(cast_option_buffer);
-   if (Z_TYPE_P(cast_option_buffer) == IS_STRING) {
-   buffer = 
estrndup(Z_STRVAL_P(cast_option_buffer), Z_STRLEN_P(cast_option_buffer));
-   buffer_len_int = 
Z_STRLEN_P(cast_option_buffer);
-   zval_ptr_dtor(cast_option_buffer);
+   zval cast_option_buffer;
+   MAKE_COPY_ZVAL(option_buffer, 
cast_option_buffer);
+   convert_to_string(cast_option_buffer);
+   if (Z_TYPE(cast_option_buffer) == IS_STRING) {
+   buffer = 
estrndup(Z_STRVAL(cast_option_buffer), Z_STRLEN(cast_option_buffer));
+   buffer_len_int = 
Z_STRLEN(cast_option_buffer);
+   zval_dtor(cast_option_buffer);
break;
}
-   zval_ptr_dtor(cast_option_buffer);
+   zval_dtor(cast_option_buffer);
}
case IS_BOOL:
case IS_NULL:


--
PHP CVS Mailing List 

Re: [PHP-CVS] com php-src: Refactor to using a stack based zval instead of dynamic allocation: ext/standard/password.c

[Nuno Lopes]

Hi,

I gave a quick review to the overal implementation of this feature.
A few comments:

- php_password_make_salt() shouldn't allocate memory + do memcpy, but it 
should fill in 'ret' directly instead. Both mallocs can go away.
- in PHP_FUNCTION(password_get_info) you assume that sscanf always 
succeeds. That's not the case if I pass a mis-encoded string.
- in PHP_FUNCTION(password_hash) you don't need to estrndup the salt, since 
you're just reading it.
- Similarly, no needs to emallocs and sprintf. You should write directly to 
the final string to avoid the copies.
- The sprintf() there is probably not ok if the salt includes a \0 in the 
middle.


In summary, there should be few or no mallocs in this file, since most 
buffers have a maximum (small) size that can be determined statically.


Nuno 



--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php