refactor: remove dead condition in session.c#21939
Conversation
jorgsowa
changed the title
| } | ||
|
|
||
| /* If there is no ID, use session module to create one */ | ||
| if (!PS(id) || !ZSTR_VAL(PS(id))[0]) { |
There was a problem hiding this comment.
if PS(id) was falsey then this branch would be taken
| @@ -446,9 +446,7 @@ | |||
| } else if (PS(use_strict_mode) && PS(mod)->s_validate_sid && | |||
| PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == FAILURE | |||
There was a problem hiding this comment.
but, is there any guarantee that s_validate_sid (which is given the PS(id) as a paremeter) doesn't make it falsey, either directly or by accessing the global?
It looks like the chain of zend_string_release_ex and on don't actually assert that the string is present - can I suggest adding a ZEND_ASSERT(PS(id)) so that if there is ever a case where the string is absent at least there is a clear failure in debug builds?
There was a problem hiding this comment.
That would be a violation of the API contract, and it requires someone to write a session storage module that purposefully fucks around with it.
I was working on trying to reduce the amount of global state in ext/session to limit those sort of problems.
3 participants
PS(id)is always non-null and non-empty when it reaches the if.