# Const-correctness - Thread: https://externals.io/message/39570 - Original archive: https://news-web.php.net/php.internals/39570 - Started by Felipe Pena on 2008-08-03T13:12:01+00:00 - Messages: 4 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #39570 — Felipe Pena — 2008-08-03T13:12:01+00:00 URL: https://externals.io/message/39570#39570 Hi all, I made a patch that add the const qualifier in several function parameters in Zend/* where it seems suitable (mostly the API). http://felipe.ath.cx/diff/const-ness.diff (5_3) HEAD comming soon in case of no objection. (And yes... I'm thinking commit it just when HEAD has been done too.) Since then, anyone have an objection? -- Regards, Felipe Pena. --- ## #39574 — Marcus Boerger — 2008-08-03T16:46:56+00:00 URL: https://externals.io/message/39570#39574 · In reply to: #39570 Hello Felipe, Sunday, August 3, 2008, 3:12:01 PM, you wrote: > Hi all, > I made a patch that add the const qualifier in several function > parameters in Zend/* where it seems suitable (mostly the API). > http://felipe.ath.cx/diff/const-ness.diff (5_3) > HEAD comming soon in case of no objection. > (And yes... I'm thinking commit it just when HEAD has been done too.) 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. -ZEND_API int zend_print_zval(zval *expr, int indent) /* {{{ */ +ZEND_API int zend_print_zval(zval * const expr, int indent) /* {{{ */ -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'. Also note that not all of them use const for expr. 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' -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'. -static int parse_arg_object_to_string(zval **arg, char **p, int *pl, int type TSRMLS_DC) /* {{{ */ +static int parse_arg_object_to_string(zval ** const arg, char ** const p, int * const pl, int type TSRMLS_DC) /* {{{ */ This is named parse. Does it convert the zval in place? I rather think it might create a copy of the zval and reduce the refcount in the original one. That would explain wht you cannot do const zval here. So at this point I guess we do that at a bunch of places and sadly we do not have mutable. Maybe we need to do a lot of casting here to make refcount and is_ref work as expected, that is not affect constness of a zval. Maybe some compilers need 'const volatile' for both. If that works you need: 'const zval ** const arg'. -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? -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'. -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'? -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. > Since then, anyone have an objection? See above, we might be able to add more const :-) Best regards, Marcus --- ## #39581 — Felipe Pena — 2008-08-03T22:06:24+00:00 URL: https://externals.io/message/39570#39581 · In reply to: #39574 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. --- ## #39590 — Moriyoshi Koizumi — 2008-08-04T01:07:00+00:00 URL: https://externals.io/message/39570#39590 · In reply to: #39574 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