Hello Marcus,

Em Dom, 2008-08-03 às 18:46 +0200, Marcus Boerger escreveu:

Index: Zend/zend.c

-static void print_hash(HashTable ht, int indent, zend_bool is_object TSRMLS_DC) / {{{ /
+static void print_hash(HashTable * const ht, int indent, zend_bool is_object TSRMLS_DC) / {{{ */
-static void print_flat_hash(HashTable ht TSRMLS_DC) / {{{ /
+static void print_flat_hash(HashTable * const ht TSRMLS_DC) / {{{ */

Why not 'const HashTable * const ht', print doesn't sound like it should modify the table itself.

That because the zend_hash_internal_pointer_reset_ex() called in
print_hash() and print_flat_hash() makes:
ht->pInternalPointer = ht->pListHead;

-ZEND_API int zend_print_zval_ex(zend_write_func_t write_func, zval expr, int indent) / {{{ */
+ZEND_API int zend_print_zval_ex(const zend_write_func_t write_func, zval expr, int indent) / {{{ */
-ZEND_API void zend_print_flat_zval_r(zval expr TSRMLS_DC) / {{{ /
+ZEND_API void zend_print_flat_zval_r(zval * const expr TSRMLS_DC) /
{{{ */
-ZEND_API void zend_print_zval_r(zval expr, int indent TSRMLS_DC) /
{{{ /
+ZEND_API void zend_print_zval_r(zval * const expr, int indent
TSRMLS_DC) / {{{ */
-ZEND_API void zend_print_zval_r_ex(zend_write_func_t write_func, zval
expr, int indent TSRMLS_DC) / {{{ */
+ZEND_API void zend_print_zval_r_ex(const zend_write_func_t
write_func, zval expr, int indent TSRMLS_DC) / {{{ */

In the same way these should be 'const zval * const expr'.

The reason is that zend_print_zval_ex() uses:
zval_dtor(expr);

Also note that not all of them use const for expr.
Oh right, I added in zend_print_zval_r_ex() now.

Index: Zend/zend_API.c

-ZEND_API int zend_copy_parameters_array(int param_count, zval argument_array TSRMLS_DC) / {{{ /
+ZEND_API int zend_copy_parameters_array(int param_count, zval * const argument_array TSRMLS_DC) / {{{ */

'const zval * const'

Fixed!

-ZEND_API int zend_get_object_classname(zval *object, char **class_name, zend_uint class_name_len TSRMLS_DC) / {{{ */
+ZEND_API int zend_get_object_classname(zval * const object, char **class_name, zend_uint class_name_len TSRMLS_DC) / {{{ */

Does not modify the zval container, so use 'const zval * const object'.

Fixed.

-static char *zend_parse_arg_impl(int arg_num, zval **arg, va_list *va, char **spec, char **error, int severity TSRMLS_DC) / {{{ */
+static char *zend_parse_arg_impl(int arg_num, zval **arg, va_list * const va, char ** const spec, char **error, int severity TSRMLS_DC) / {{{ */
-static int zend_parse_arg(int arg_num, zval **arg, va_list *va, char *spec, int quiet TSRMLS_DC) / {{{ */
+static int zend_parse_arg(int arg_num, zval *arg, va_list * const va, char ** const spec, int quiet TSRMLS_DC) / {{{ */

Shouldn't this be 'const char ** const spec'. After all we don't modify the actual string.
-static int zend_parse_va_args(int num_args, char *type_spec, va_list va, int flags TSRMLS_DC) / {{{ */
+static int zend_parse_va_args(int num_args, char type_spec, va_list * const va, int flags TSRMLS_DC) / {{{ */

Shouldn't this be 'const char *type_spec' for the same reason as above?

Fixed!

-ZEND_API int zend_parse_parameters_ex(int flags, int num_args TSRMLS_DC, char type_spec, ...) / {{{ /
+ZEND_API int zend_parse_parameters_ex(int flags, int num_args TSRMLS_DC, char * const type_spec, ...) / {{{ */
-ZEND_API int zend_parse_parameters(int num_args TSRMLS_DC, char type_spec, ...) / {{{ /
+ZEND_API int zend_parse_parameters(int num_args TSRMLS_DC, char * const type_spec, ...) / {{{ */
-ZEND_API int zend_parse_method_parameters(int num_args TSRMLS_DC, zval *this_ptr, char type_spec, ...) / {{{ /
+ZEND_API int zend_parse_method_parameters(int num_args TSRMLS_DC, zval * const this_ptr, char * const type_spec, ...) / {{{ */
-ZEND_API int zend_parse_method_parameters_ex(int flags, int num_args TSRMLS_DC, zval *this_ptr, char type_spec, ...) / {{{ /
+ZEND_API int zend_parse_method_parameters_ex(int flags, int num_args TSRMLS_DC, zval * const this_ptr, char * const type_spec, ...) / {{{ */

Shouldn't this be 'const char * const type_spec'.
For the method versions it should also be 'const zval * const this_ptr'.

Fixed!

-ZEND_API int zend_startup_module_ex(zend_module_entry module TSRMLS_DC) / {{{ /
+ZEND_API int zend_startup_module_ex(zend_module_entry * const module TSRMLS_DC) / {{{ */

Why not 'const zend_module_entry * const module'?

Because module->module_started had your value changed in the function.

-ZEND_API zend_module_entry* zend_register_internal_module(zend_module_entry module TSRMLS_DC) / {{{ /
+ZEND_API zend_module_entry zend_register_internal_module(zend_module_entry * const module TSRMLS_DC) /* {{{ */

If only we had mutable...

Overall you do a ton of 'struct * const var' which only means that you are
going to copy the pointer explicitly. Now functions that use a pointer in a
loop and increment it cannot optimize the code anymore and are forced to use
an additional real variable on the stack. So unless you have a very smart
compiler, the result is an increased stack size. Generally speaking I prefer
const on the reight and mid (between *'s) only. But others prefer it to denote
that not even the passed in pointer gets modifed and this sometimes even makes
debugging easier.

Hmmm, ok. I'll remove all the 'struct * const var', and to find other
cases where 'const struct * const var' and 'const var' can be suitable
too.

const thanks! :D

--
Regards,
Felipe Pena.

Marcus Boerger wrote:

Overall you do a ton of 'struct * const var' which only means that you are
going to copy the pointer explicitly. Now functions that use a pointer in a
loop and increment it cannot optimize the code anymore and are forced to use
an additional real variable on the stack. So unless you have a very smart
compiler, the result is an increased stack size. Generally speaking I prefer
const on the reight and mid (between *'s) only. But others prefer it to denote
that not even the passed in pointer gets modifed and this sometimes even makes
debugging easier.

That's not necessarily true. In contrast to the volatile qualifier, the
const qualifier in this context is just a semantic thing. So if your
function has a immutable argument variable, it probably gets compiled a
procedure that receives the argument in the registers and use it
mutatively as long as no side-effects are possible. (of cource how it
compiles depends on the target platform)

Having that said, I don't think it makes any sense to make pointer
variables immutable, while marking the referent immutable greatly
reduces tiny errors. Compilers are often smarter in this case.

Moriyoshi

Moriyoshi Koizumi <moriyoshi@at.wakwak.com