ext/opcache: minor refactoring (2026-03)#21397
Conversation
| 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; |
There was a problem hiding this comment.
This is a functional change. Previously a non-falsy value was returned.
There was a problem hiding this comment.
Yeah this should likely return true. It will prevent shm from being used or added to during a restart.
There was a problem hiding this comment.
Okay, will fix it but seeing failure I assumed it should be false.
arnaud-lb
left a comment
arnaud-lb
left a comment
There was a problem hiding this comment.
Looks good to me apart from Tim's comment
iluuu1994
left a comment
iluuu1994
left a comment
There was a problem hiding this comment.
Looks ok to me, apart from the comment made by Tim.
| 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); |
There was a problem hiding this comment.
Nit: Technically the name dependent_ce indicates a dependency of dependent_ce on ce, rather than the other way around.
There was a problem hiding this comment.
Do you have an idea for a better name? dependency_of_ce maybe?
| 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; |
There was a problem hiding this comment.
Yeah this should likely return true. It will prevent shm from being used or added to during a restart.
4 participants
Commits should be reviewed in order.