The **const** modifier needs to be added to read-only parameters of all ZendAPI and PHPAPI functions

[matyhtf]

Description

Many functions in the php source code receive the zval parameter. The value of the parameter is not modified in the function implementation. These parameters are read-only, but the const modifier is not added.

As a result, C/C++underlying developers do not know whether the function is a read-only operation or a write operation, and will pollute all downstream functions. If these PHPAPI or ZEND_API functions are used in a C++function, even if the function is designed to be read-only, the const modifier cannot be used, or only unsafe const_cast can be used to cast.

An example:

class Variant {
  protected:
    zval val;
    void destroy() {
        zval_ptr_dtor(&val);
    }

  public:
    Variant(const zval *v) noexcept {
        ZVAL_COPY_VALUE(&val, v);
    }
    const zval *const_ptr() const {
        return &val;
    }
    // fast_equal_check_function should be `fast_equal_check_function(const zval *op1, const zval *op2)`
     bool equals(const Variant &v) const {
        return fast_equal_check_function(const_ptr(), v.const_ptr());         
    }
}

Problem function:

• fast_equal_check_function
• php_debug_zval_dump
• php_var_dump
• php_var_unserialize
• concat_function
• add_function
• is_equal_function
• zval_get_string
• zval_get_double // version 8.1

... a lot more functions

[DanielEScherzer]

So I'm not sure that the list of functions given is accurate, e.g.

php-src/Zend/zend_operators.h

Lines 325 to 327 in 63acc4b

	static zend_always_inline zend_string *zval_get_string(zval *op) {
	return EXPECTED(Z_TYPE_P(op) == IS_STRING) ? zend_string_copy(Z_STR_P(op)) : zval_get_string_func(op);
	}

php-src/Zend/zend_string.h

Lines 207 to 213 in 63acc4b

	static zend_always_inline zend_string *zend_string_copy(zend_string *s)
	{
	if (!ZSTR_IS_INTERNED(s)) {
	GC_ADDREF(s);
	}
	return s;
	}

zvals for strings may have the pointed to memory for the underlying string updated (and for non-strings things might be worse, e.g. for objects calling the object's __toString() means that arbitrary user code might be run). Is the parameter still read-only if it contains a pointer to memory that might change?

[bwoebi]

Is the parameter still read-only if it contains a pointer to memory that might change?

Yes. const only applies to the direct pointee.

[iluuu1994]

@matyhtf Feel free to send PRs for master. 🙂 Generally, adding const to parameters should be ok. There's a small risk of ABI-incompatibility if for some reason we need to remove the const in a patch to solve a problem, but this isn't very likely.