ext/phar: assert function are not passed NULL pointers#19356
Conversation
Girgias
force-pushed
the
phar-spprintf-errors-refacto
branch
2 times, most recently
from
dabaffb to
c618444
Compare
There was a problem hiding this comment.
Looks mainly good. For the future it would be good to defer zend_string refactors to another PR because it's just another thing to check, and I already had to reason "recursively" to check the validity for these changes, so less context switching is better
Girgias
force-pushed
the
phar-spprintf-errors-refacto
branch
from
c618444 to
6b0acde
Compare
|
CI failure is legit. It's probably because of code that was added/changed in the meantime. |
Yeah, but I wanted to extract some of the changes into a different PR which is now done in #20162 I'll rebase after that is merged and fix the NULL pointer attributes :) |
Girgias
force-pushed
the
phar-spprintf-errors-refacto
branch
from
6b0acde to
4e923ee
Compare
Girgias
force-pushed
the
phar-spprintf-errors-refacto
branch
from
4e923ee to
4892ece
Compare
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
The test changes and C changes should likely be committed separately
|
Right, I forgot about them as I used them to debug my test failures. But those were just caused due to my OpenSSL not being able to verify the signature |
This simplifies some of the logic and makes the assumptions clear
Girgias
force-pushed
the
phar-spprintf-errors-refacto
branch
from
4892ece to
aa3d4d5
Compare
2 participants
This simplifies some of the logic and makes the assumptions clear