Skip to content

ext/opcache: minor refactoring (2026-03) #21397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Girgias wants to merge 6 commits into
base: master
Choose a base branch
from
+39 −43
Open

ext/opcache: minor refactoring (2026-03) #21397

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eab2d11
ext/opcache: formalize return type of accel_restart_is_active() to bool
Girgias Mar 9, 2026
d558d84
ext/opcache: formalize return type of zend_get_stream_timestamp() to …
Girgias Mar 9, 2026
59c1eb8
ext/opcache: formalize return type of do_validate_timestamps() to zen…
Girgias Mar 9, 2026
80df36f
ext/opcache: formalize return type of check_persistent_script_access(…
Girgias Mar 9, 2026
1777fad
ext/opcache: reduce scope of variable
Girgias Mar 9, 2026
ae63da1
ext/opcache: add const qualifiers
Girgias Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 39 additions & 43 deletions ext/opcache/ZendAccelerator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ static inline void accel_restart_leave(void)
#endif
}

static inline int accel_restart_is_active(void)
static inline bool accel_restart_is_active(void)
{
if (ZCSG(restart_in_progress)) {
#ifndef ZEND_WIN32
Expand All @@ -325,19 +325,19 @@ static inline int accel_restart_is_active(void)

if (fcntl(lock_file, F_GETLK, &restart_check) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC: %s (%d)", strerror(errno), errno);
return FAILURE;
return false;
Copy link
Member

@TimWolla TimWolla Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a functional change. Previously a non-falsy value was returned.

Copy link
Member

@iluuu1994 iluuu1994 Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this should likely return true. It will prevent shm from being used or added to during a restart.

Copy link
Member Author

@Girgias Girgias Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, will fix it but seeing failure I assumed it should be false.

}
if (restart_check.l_type == F_UNLCK) {
ZCSG(restart_in_progress) = false;
return 0;
return false;
} else {
return 1;
return true;
}
#else
return LOCKVAL(restart_in) != 0;
#endif
}
return 0;
return false;
}

/* Creates a read lock for SHM access */
Expand Down Expand Up @@ -591,13 +591,12 @@ static zend_string* ZEND_FASTCALL accel_new_interned_string_for_php(zend_string
static zend_always_inline zend_string *accel_find_interned_string_ex(zend_ulong h, const char *str, size_t size)
{
zend_string_table_pos_t pos;
zend_string *s;

/* check for existing interned string */
pos = *STRTAB_HASH_TO_SLOT(&ZCSG(interned_strings), h);
if (EXPECTED(pos != STRTAB_INVALID_POS)) {
do {
s = STRTAB_POS_TO_STR(&ZCSG(interned_strings), pos);
zend_string *s = STRTAB_POS_TO_STR(&ZCSG(interned_strings), pos);
if (EXPECTED(ZSTR_H(s) == h) && zend_string_equals_cstr(s, str, size)) {
return s;
}
Expand Down Expand Up @@ -720,14 +719,13 @@ static void accel_copy_permanent_strings(zend_new_interned_string_func_t new_int
Z_FUNC(q->val)->common.function_name = new_interned_string(Z_FUNC(q->val)->common.function_name);
}
if (Z_FUNC(q->val)->common.scope == ce) {
uint32_t i;
uint32_t num_args = Z_FUNC(q->val)->common.num_args + 1;
zend_arg_info *arg_info = Z_FUNC(q->val)->common.arg_info - 1;

if (Z_FUNC(q->val)->common.fn_flags & ZEND_ACC_VARIADIC) {
num_args++;
}
for (i = 0 ; i < num_args; i++) {
for (uint32_t i = 0 ; i < num_args; i++) {
if (i > 0) {
arg_info[i].name = new_interned_string(arg_info[i].name);
if (arg_info[i].default_value) {
Expand Down Expand Up @@ -964,7 +962,7 @@ static inline bool accel_is_inactive(void)
return false;
}

static int zend_get_stream_timestamp(const char *filename, zend_stat_t *statbuf)
static zend_result zend_get_stream_timestamp(const char *filename, zend_stat_t *statbuf)
{
php_stream_wrapper *wrapper;
php_stream_statbuf stream_statbuf;
Expand Down Expand Up @@ -1137,7 +1135,7 @@ accel_time_t zend_get_file_handle_timestamp(zend_file_handle *file_handle, size_
return statbuf.st_mtime;
}

static inline int do_validate_timestamps(zend_persistent_script *persistent_script, zend_file_handle *file_handle)
static inline zend_result do_validate_timestamps(const zend_persistent_script *persistent_script, zend_file_handle *file_handle)
{
zend_file_handle ps_handle;
zend_string *full_path_ptr = NULL;
Expand Down Expand Up @@ -1523,7 +1521,7 @@ static void zend_accel_add_key(zend_string *key, zend_accel_hash_entry *bucket)
}
}

static zend_always_inline bool is_phar_file(zend_string *filename)
static zend_always_inline bool is_phar_file(const zend_string *filename)
{
return filename && ZSTR_LEN(filename) >= sizeof(".phar") &&
!memcmp(ZSTR_VAL(filename) + ZSTR_LEN(filename) - (sizeof(".phar")-1), ".phar", sizeof(".phar")-1) &&
Expand Down Expand Up @@ -1993,7 +1991,7 @@ static zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int
return op_array;
}

static int check_persistent_script_access(zend_persistent_script *persistent_script)
static bool check_persistent_script_access(const zend_persistent_script *persistent_script)
{
char *phar_path, *ptr;
if ((ZSTR_LEN(persistent_script->script.filename)<sizeof("phar://.phar")) ||
Expand Down Expand Up @@ -2294,7 +2292,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
return zend_accel_load_script(persistent_script, from_shared_memory);
}

static zend_always_inline zend_inheritance_cache_entry* zend_accel_inheritance_cache_find(zend_inheritance_cache_entry *entry, zend_class_entry *ce, zend_class_entry *parent, zend_class_entry **traits_and_interfaces, bool *needs_autoload_ptr)
static zend_always_inline zend_inheritance_cache_entry* zend_accel_inheritance_cache_find(zend_inheritance_cache_entry *entry, const zend_class_entry *ce, const zend_class_entry *parent, zend_class_entry **traits_and_interfaces, bool *needs_autoload_ptr)
{
uint32_t i;

Expand All @@ -2316,10 +2314,10 @@ static zend_always_inline zend_inheritance_cache_entry* zend_accel_inheritance_c
}
if (found && entry->dependencies) {
for (i = 0; i < entry->dependencies_count; i++) {
zend_class_entry *ce = zend_lookup_class_ex(entry->dependencies[i].name, NULL, ZEND_FETCH_CLASS_NO_AUTOLOAD);
const zend_class_entry *dependent_ce = zend_lookup_class_ex(entry->dependencies[i].name, NULL, ZEND_FETCH_CLASS_NO_AUTOLOAD);

if (ce != entry->dependencies[i].ce) {
if (!ce) {
if (dependent_ce != entry->dependencies[i].ce) {
if (!dependent_ce) {
needs_autoload = true;
} else {
found = false;
Expand All @@ -2341,7 +2339,6 @@ static zend_always_inline zend_inheritance_cache_entry* zend_accel_inheritance_c

static zend_class_entry* zend_accel_inheritance_cache_get(zend_class_entry *ce, zend_class_entry *parent, zend_class_entry **traits_and_interfaces)
{
uint32_t i;
bool needs_autoload;
zend_inheritance_cache_entry *entry = ce->inheritance_cache;

Expand All @@ -2360,10 +2357,10 @@ static zend_class_entry* zend_accel_inheritance_cache_get(zend_class_entry *ce,
return ce;
}

for (i = 0; i < entry->dependencies_count; i++) {
zend_class_entry *ce = zend_lookup_class_ex(entry->dependencies[i].name, NULL, 0);
for (uint32_t i = 0; i < entry->dependencies_count; i++) {
const zend_class_entry *dependent_ce = zend_lookup_class_ex(entry->dependencies[i].name, NULL, 0);
Copy link
Member

@iluuu1994 iluuu1994 Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Technically the name dependent_ce indicates a dependency of dependent_ce on ce, rather than the other way around.

Copy link
Member Author

@Girgias Girgias Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an idea for a better name? dependency_of_ce maybe?


if (ce == NULL) {
if (dependent_ce == NULL) {
return NULL;
}
}
Expand Down Expand Up @@ -3649,7 +3646,7 @@ static void preload_shutdown(void)

if (EG(function_table)) {
ZEND_HASH_MAP_REVERSE_FOREACH_VAL(EG(function_table), zv) {
zend_function *func = Z_PTR_P(zv);
const zend_function *func = Z_PTR_P(zv);
if (func->type == ZEND_INTERNAL_FUNCTION) {
break;
}
Expand All @@ -3658,7 +3655,7 @@ static void preload_shutdown(void)

if (EG(class_table)) {
ZEND_HASH_MAP_REVERSE_FOREACH_VAL(EG(class_table), zv) {
zend_class_entry *ce = Z_PTR_P(zv);
const zend_class_entry *ce = Z_PTR_P(zv);
if (ce->type == ZEND_INTERNAL_CLASS && Z_TYPE_P(zv) != IS_ALIAS_PTR) {
break;
}
Expand All @@ -3685,7 +3682,7 @@ static void preload_restart(void)
}
}

static size_t preload_try_strip_filename(zend_string *filename) {
static size_t preload_try_strip_filename(const zend_string *filename) {
/*FIXME: better way to handle eval()'d code? see COMPILED_STRING_DESCRIPTION_FORMAT */
if (ZSTR_LEN(filename) > sizeof(" eval()'d code")
&& *(ZSTR_VAL(filename) + ZSTR_LEN(filename) - sizeof(" eval()'d code")) == ':') {
Expand Down Expand Up @@ -3806,17 +3803,17 @@ static void preload_sort_classes(void *base, size_t count, size_t siz, compare_f
Bucket *b2;
Bucket *end = b1 + count;
Bucket tmp;
zend_class_entry *ce, *p;
const zend_class_entry *ce, *p;

while (b1 < end) {
try_again:
ce = (zend_class_entry*)Z_PTR(b1->val);
ce = Z_PTR(b1->val);
if (ce->parent && (ce->ce_flags & ZEND_ACC_LINKED)) {
p = ce->parent;
if (p->type == ZEND_USER_CLASS) {
b2 = b1 + 1;
while (b2 < end) {
if (p == Z_PTR(b2->val)) {
if (p == Z_PTR(b2->val)) {
tmp = *b1;
*b1 = *b2;
*b2 = tmp;
Expand All @@ -3833,7 +3830,7 @@ static void preload_sort_classes(void *base, size_t count, size_t siz, compare_f
if (p->type == ZEND_USER_CLASS) {
b2 = b1 + 1;
while (b2 < end) {
if (p == Z_PTR(b2->val)) {
if (p == Z_PTR(b2->val)) {
tmp = *b1;
*b1 = *b2;
*b2 = tmp;
Expand All @@ -3859,7 +3856,7 @@ static zend_result preload_resolve_deps(preload_error *error, const zend_class_e

if (ce->parent_name) {
zend_string *key = zend_string_tolower(ce->parent_name);
zend_class_entry *parent = zend_hash_find_ptr(EG(class_table), key);
const zend_class_entry *parent = zend_hash_find_ptr(EG(class_table), key);
zend_string_release(key);
if (!parent) {
error->kind = "Unknown parent ";
Expand All @@ -3870,7 +3867,7 @@ static zend_result preload_resolve_deps(preload_error *error, const zend_class_e

if (ce->num_interfaces) {
for (uint32_t i = 0; i < ce->num_interfaces; i++) {
zend_class_entry *interface =
const zend_class_entry *interface =
zend_hash_find_ptr(EG(class_table), ce->interface_names[i].lc_name);
if (!interface) {
error->kind = "Unknown interface ";
Expand All @@ -3882,7 +3879,7 @@ static zend_result preload_resolve_deps(preload_error *error, const zend_class_e

if (ce->num_traits) {
for (uint32_t i = 0; i < ce->num_traits; i++) {
zend_class_entry *trait =
const zend_class_entry *trait =
zend_hash_find_ptr(EG(class_table), ce->trait_names[i].lc_name);
if (!trait) {
error->kind = "Unknown trait ";
Expand Down Expand Up @@ -3925,10 +3922,9 @@ static bool preload_try_resolve_constants(zend_class_entry *ce)
ce->ce_flags &= ~ZEND_ACC_HAS_AST_CONSTANTS;
}
if (ce->default_properties_count) {
uint32_t i;
bool resolved = true;

for (i = 0; i < ce->default_properties_count; i++) {
for (uint32_t i = 0; i < ce->default_properties_count; i++) {
zend_property_info *prop = ce->properties_info_table[i];
if (!prop) {
continue;
Expand Down Expand Up @@ -3988,7 +3984,7 @@ static void preload_error_cb(int type, zend_string *error_filename, const uint32
static void preload_remove_declares(zend_op_array *op_array)
{
zend_op *opline = op_array->opcodes;
zend_op *end = opline + op_array->last;
const zend_op *end = opline + op_array->last;
uint32_t skip_dynamic_func_count = 0;
zend_string *key;
zend_op_array *func;
Expand Down Expand Up @@ -4320,7 +4316,7 @@ static void preload_remove_empty_includes(void)
/* remove empty includes */
ZEND_HASH_MAP_FOREACH_PTR(preload_scripts, script) {
zend_op *opline = script->script.main_op_array.opcodes;
zend_op *end = opline + script->script.main_op_array.last;
const zend_op *end = opline + script->script.main_op_array.last;

while (opline < end) {
if (opline->opcode == ZEND_INCLUDE_OR_EVAL &&
Expand Down Expand Up @@ -4349,7 +4345,7 @@ static void preload_remove_empty_includes(void)
} ZEND_HASH_FOREACH_END();
}

static void preload_register_trait_methods(zend_class_entry *ce) {
static void preload_register_trait_methods(const zend_class_entry *ce) {
zend_op_array *op_array;
zend_property_info *info;

Expand Down Expand Up @@ -4383,7 +4379,7 @@ static void preload_fix_trait_op_array(zend_op_array *op_array)
return;
}

zend_op_array *orig_op_array = zend_shared_alloc_get_xlat_entry(op_array->refcount);
const zend_op_array *orig_op_array = zend_shared_alloc_get_xlat_entry(op_array->refcount);
ZEND_ASSERT(orig_op_array && "Must be in xlat table");

zend_string *function_name = op_array->function_name;
Expand All @@ -4401,7 +4397,7 @@ static void preload_fix_trait_op_array(zend_op_array *op_array)
op_array->static_variables = ht;
}

static void preload_fix_trait_methods(zend_class_entry *ce)
static void preload_fix_trait_methods(const zend_class_entry *ce)
{
zend_op_array *op_array;

Expand All @@ -4426,7 +4422,7 @@ static void preload_fix_trait_methods(zend_class_entry *ce)

static void preload_optimize(zend_persistent_script *script)
{
zend_class_entry *ce;
const zend_class_entry *ce;
zend_persistent_script *tmp_script;

zend_shared_alloc_init_xlat_table();
Expand Down Expand Up @@ -4537,7 +4533,7 @@ static void preload_load(size_t orig_map_ptr_static_last)

if (zend_hash_num_elements(&script->class_table)) {
Bucket *p = script->class_table.arData;
Bucket *end = p + script->class_table.nNumUsed;
const Bucket *end = p + script->class_table.nNumUsed;

zend_hash_extend(CG(class_table),
CG(class_table)->nNumUsed + script->class_table.nNumUsed, 0);
Expand Down Expand Up @@ -4585,7 +4581,7 @@ static void preload_load(size_t orig_map_ptr_static_last)
}

#if HAVE_JIT
static void zend_accel_clear_call_graph_ptrs(zend_op_array *op_array)
static void zend_accel_clear_call_graph_ptrs(const zend_op_array *op_array)
{
ZEND_ASSERT(ZEND_USER_CODE(op_array->type));
zend_func_info *info = ZEND_FUNC_INFO(op_array);
Expand All @@ -4595,9 +4591,9 @@ static void zend_accel_clear_call_graph_ptrs(zend_op_array *op_array)
}
}

static void accel_reset_arena_info(zend_persistent_script *script)
static void accel_reset_arena_info(const zend_persistent_script *script)
{
zend_op_array *op_array;
const zend_op_array *op_array;
zend_class_entry *ce;

zend_accel_clear_call_graph_ptrs(&script->script.main_op_array);
Expand Down
Loading