ext/standard/stream: Use FCC instead of zval for notification callback #19024
Conversation
Also check that the callable exists while setting the option
ext/standard/streamsfuncs.c
Outdated
| } | ||
| for (i = 0; i < 6; i++) { | ||
| zend_call_known_fcc(context->notifier->fcc, NULL, 6, zvs, NULL); | ||
| for (uint8_t i = 0; i < 6; i++) { |
There was a problem hiding this comment.
Fun fact: If not unrolled, using an (unsigned) int would be better in the sense that the generated assembly for both x86-64 and arm64 is shorter. For x86 it saves an instruction prefix byte to switch to 8 bit mode, and for arm64 it saves an instruction.
However, the loop is likely unrolled in which case it doesn't matter, but just something to take into account that using a shorter type is not always better.
There was a problem hiding this comment.
Actually, why do we even have a loop? Just call the zval_ptr_dtor(_str) on &zvs[2] ?
There was a problem hiding this comment.
I was thinking about that, but can't this be an issue if the notifier callback takes the arguments by-ref and changes them to strings?
There was a problem hiding this comment.
No, because the conversion happens on the call frame, not on the original zvals.
Also check that the callable exists while setting the option