archive/php-src
1
0
Fork 0
mirror of https://github.com/php/php-src.git synced 2025-08-16 05:58:45 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Clean up strtok implementation

Browse source 
Store the zend_string instead of performing a copy and storing
in a zval. Also make sure the string is released immediately if
it's no longer needed. Finally, avoid null pointer offset UB if
no string has been set -- though I'm wondering if this case
shouldn't be generating a warning?
This commit is contained in:
Nikita Popov 2020-08-27 12:24:58 +02:00
parent 0026d8a783
commit ce83ec8790
3 changed files with 23 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

6
ext/standard/basic_functions.c
View file

@ -458,7 +458,6 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */
memset(&BG(unserialize), 0, sizeof(BG(unserialize)));
BG(strtok_string) = NULL;
ZVAL_UNDEF(&BG(strtok_zval));
BG(strtok_last) = NULL;
BG(ctype_string) = NULL;
BG(locale_changed) = 0;
@ -497,9 +496,10 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */
PHP_RSHUTDOWN_FUNCTION(basic) /* {{{ */
{
zval_ptr_dtor(&BG(strtok_zval));
ZVAL_UNDEF(&BG(strtok_zval));
if (BG(strtok_string)) {
zend_string_release(BG(strtok_string));
BG(strtok_string) = NULL;
}
#ifdef HAVE_PUTENV
tsrm_env_lock();
zend_hash_destroy(&BG(putenv_ht));

3
ext/standard/basic_functions.h
View file

@ -60,8 +60,7 @@ typedef int32_t php_int32;
typedef struct _php_basic_globals {
HashTable *user_shutdown_function_names;
HashTable putenv_ht;
zval strtok_zval;
char *strtok_string;
zend_string *strtok_string;
zend_string *ctype_string; /* current LC_CTYPE locale (or NULL for 'C') */
zend_bool locale_changed; /* locale was changed and has to be restored */
char *strtok_last;

30
ext/standard/string.c
View file

@ -1271,16 +1271,24 @@ PHP_FUNCTION(strtok)
if (ZEND_NUM_ARGS() == 1) {
tok = str;
} else {
zval_ptr_dtor(&BG(strtok_zval));
ZVAL_STRINGL(&BG(strtok_zval), ZSTR_VAL(str), ZSTR_LEN(str));
BG(strtok_last) = BG(strtok_string) = Z_STRVAL(BG(strtok_zval));
if (BG(strtok_string)) {
zend_string_release(BG(strtok_string));
}
BG(strtok_string) = zend_string_copy(str);
BG(strtok_last) = ZSTR_VAL(str);
BG(strtok_len) = ZSTR_LEN(str);
}
p = BG(strtok_last); /* Where we start to search */
pe = BG(strtok_string) + BG(strtok_len);
if (!BG(strtok_string)) {
/* String to tokenize not set. */
// TODO: Should this warn?
RETURN_FALSE;
}
if (!p || p >= pe) {
p = BG(strtok_last); /* Where we start to search */
pe = ZSTR_VAL(BG(strtok_string)) + BG(strtok_len);
if (p >= pe) {
/* Reached the end of the string. */
RETURN_FALSE;
}
@ -1295,9 +1303,7 @@ PHP_FUNCTION(strtok)
while (STRTOK_TABLE(p)) {
if (++p >= pe) {
/* no other chars left */
BG(strtok_last) = NULL;
RETVAL_FALSE;
goto restore;
goto return_false;
}
skipped++;
}
@ -1314,14 +1320,14 @@ return_token:
RETVAL_STRINGL(BG(strtok_last) + skipped, (p - BG(strtok_last)) - skipped);
BG(strtok_last) = p + 1;
} else {
return_false:
RETVAL_FALSE;
BG(strtok_last) = NULL;
zend_string_release(BG(strtok_string));
BG(strtok_string) = NULL;
}
/* Restore table -- usually faster then memset'ing the table on every invocation */
restore:
token = ZSTR_VAL(tok);
while (token < token_end) {
STRTOK_TABLE(token++) = 0;
}