Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext/standard: Some more small refactoring #15391

Merged
merged 7 commits into from
Sep 12, 2024
Merged

ext/standard: Some more small refactoring #15391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f25e9ce
ext/standard/quot_print.c: Minor refactoring to php_hex2int()
Girgias Aug 13, 2024
f74483c
ext/standard/quot_print.c: Mark readonly string as const
Girgias Aug 13, 2024
05cc321
ext/standard/string.c: Refactor php_spn_common_handler()
Girgias Aug 13, 2024
578659c
ext/standard/string.c: Remove to(upper|lower) PHP API in favour of Ze…
Girgias Aug 13, 2024
60a585a
ext/standard/type.c: Remove unused include
Girgias Aug 13, 2024
07ce1b5
ext/standard/url.c: Stop exposing php_replace_controlchars_ex()
Girgias Aug 13, 2024
74330b9
ext/standard/versioning.c: Slightly refactor compare_special_version_…
Girgias Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADING.INTERNALS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,11 @@ PHP 8.4 INTERNALS UPGRADE NOTES
- The deprecated php_uint32 and php_int32 typedefs have been removed from
ext/standard/basic_functions.h. Use the standard uint32_t and int32_t
types instead.
- The php_strtoupper(), php_string_toupper(), php_strtolower(), and
php_string_tolower() functions has been removed, use zend_str_toupper(),
zend_string_toupper(), zend_str_tolower(), and zend_string_tolower()
respectively instead.
- The php_replace_controlchars_ex() function is no longer exposed.

h. ext/session
- Added the php_get_session_status() API to get the session status, which is
Expand Down
7 changes: 0 additions & 7 deletions ext/standard/php_string.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ PHP_MINIT_FUNCTION(string_intrin);
strnatcmp_ex(a, strlen(a), b, strlen(b), 1)
PHPAPI int strnatcmp_ex(char const *a, size_t a_len, char const *b, size_t b_len, bool is_case_insensitive);
PHPAPI struct lconv *localeconv_r(struct lconv *out);
PHPAPI char *php_strtoupper(char *s, size_t len);
PHPAPI char *php_strtolower(char *s, size_t len);
PHPAPI zend_string *php_string_toupper(zend_string *s);
PHPAPI zend_string *php_string_tolower(zend_string *s);
PHPAPI char *php_strtr(char *str, size_t len, const char *str_from, const char *str_to, size_t trlen);
PHPAPI zend_string *php_addslashes(zend_string *str);
PHPAPI void php_stripslashes(zend_string *str);
Expand Down Expand Up @@ -80,7 +76,4 @@ PHPAPI bool php_binary_string_shuffle(php_random_algo_with_state engine, char *s
#define PHP_PATHINFO_FILENAME 8
#define PHP_PATHINFO_ALL (PHP_PATHINFO_DIRNAME | PHP_PATHINFO_BASENAME | PHP_PATHINFO_EXTENSION | PHP_PATHINFO_FILENAME)

#define PHP_STR_STRSPN 0
#define PHP_STR_STRCSPN 1

#endif /* PHP_STRING_H */
12 changes: 4 additions & 8 deletions ext/standard/quot_print.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,12 @@ static char php_hex2int(int c) /* {{{ */
{
if (isdigit(c)) {
return c - '0';
}
else if (c >= 'A' && c <= 'F') {
} else if (c >= 'A' && c <= 'F') {
return c - 'A' + 10;
}
else if (c >= 'a' && c <= 'f') {
} else {
ZEND_ASSERT(c >= 'a' && c <= 'f');
return c - 'a' + 10;
}
else {
return -1;
}
Comment on lines +37 to -46
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should not just use a bitwise operation to make it unconditionally lower/uppercase so that we don't need to do another if check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be probably more expensive if I understand correct you want to use store (not just load). What you can do is is maybe to get rid of one condition but not sure if it's worth the readability:

if (isdigit(c)) {
	return c - '0';
} else if (c >= 'a') {
	ZEND_ASSERT(c <= 'f');
	return c - 'a' + 10;
} else {
	ZEND_ASSERT(c >= 'A' && c <= 'F');
	return c - 'A' + 10;
}

}
/* }}} */

Expand Down Expand Up @@ -145,7 +141,7 @@ PHPAPI zend_string *php_quot_print_encode(const unsigned char *str, size_t lengt
{
zend_ulong lp = 0;
unsigned char c, *d;
char *hex = "0123456789ABCDEF";
const char *hex = "0123456789ABCDEF";
zend_string *ret;

ret = zend_string_safe_alloc(3, (length + (((3 * length)/(PHP_QPRINT_MAXL-9)) + 1)), 0, 0);
Expand Down
39 changes: 4 additions & 35 deletions ext/standard/string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ PHP_FUNCTION(hex2bin)
}
/* }}} */

static void php_spn_common_handler(INTERNAL_FUNCTION_PARAMETERS, int behavior) /* {{{ */
static void php_spn_common_handler(INTERNAL_FUNCTION_PARAMETERS, bool is_strspn) /* {{{ */
{
zend_string *s11, *s22;
zend_long start = 0, len = 0;
Expand Down Expand Up @@ -249,13 +249,12 @@ static void php_spn_common_handler(INTERNAL_FUNCTION_PARAMETERS, int behavior) /
RETURN_LONG(0);
}

if (behavior == PHP_STR_STRSPN) {
if (is_strspn) {
RETURN_LONG(php_strspn(ZSTR_VAL(s11) + start /*str1_start*/,
ZSTR_VAL(s22) /*str2_start*/,
ZSTR_VAL(s11) + start + len /*str1_end*/,
ZSTR_VAL(s22) + ZSTR_LEN(s22) /*str2_end*/));
} else {
ZEND_ASSERT(behavior == PHP_STR_STRCSPN);
RETURN_LONG(php_strcspn(ZSTR_VAL(s11) + start /*str1_start*/,
ZSTR_VAL(s22) /*str2_start*/,
ZSTR_VAL(s11) + start + len /*str1_end*/,
Expand All @@ -267,14 +266,14 @@ static void php_spn_common_handler(INTERNAL_FUNCTION_PARAMETERS, int behavior) /
/* {{{ Finds length of initial segment consisting entirely of characters found in mask. If start or/and length is provided works like strspn(substr($s,$start,$len),$good_chars) */
PHP_FUNCTION(strspn)
{
php_spn_common_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_STR_STRSPN);
php_spn_common_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU, /* is_strspn */ true);
}
/* }}} */

/* {{{ Finds length of initial segment consisting entirely of characters not found in mask. If start or/and length is provide works like strcspn(substr($s,$start,$len),$bad_chars) */
PHP_FUNCTION(strcspn)
{
php_spn_common_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU, PHP_STR_STRCSPN);
php_spn_common_handler(INTERNAL_FUNCTION_PARAM_PASSTHRU, /* is_strspn */ false);
}
/* }}} */

Expand Down Expand Up @@ -1212,21 +1211,6 @@ PHP_FUNCTION(strtok)
}
/* }}} */

/* {{{ php_strtoupper */
PHPAPI char *php_strtoupper(char *s, size_t len)
{
zend_str_toupper(s, len);
return s;
}
/* }}} */

/* {{{ php_string_toupper */
PHPAPI zend_string *php_string_toupper(zend_string *s)
{
return zend_string_toupper(s);
}
/* }}} */

/* {{{ Makes a string uppercase */
PHP_FUNCTION(strtoupper)
{
Expand All @@ -1240,21 +1224,6 @@ PHP_FUNCTION(strtoupper)
}
/* }}} */

/* {{{ php_strtolower */
PHPAPI char *php_strtolower(char *s, size_t len)
{
zend_str_tolower(s, len);
return s;
}
/* }}} */

/* {{{ php_string_tolower */
PHPAPI zend_string *php_string_tolower(zend_string *s)
{
return zend_string_tolower(s);
}
/* }}} */

/* {{{ Makes a string lowercase */
PHP_FUNCTION(strtolower)
{
Expand Down
1 change: 0 additions & 1 deletion ext/standard/type.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

#include "php.h"
#include "php_incomplete_class.h"

/* {{{ Returns the type of the variable */
PHP_FUNCTION(gettype)
Expand Down
36 changes: 12 additions & 24 deletions ext/standard/url.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,19 @@ PHPAPI void php_url_free(php_url *theurl)
}
/* }}} */

/* {{{ php_replace_controlchars_ex */
PHPAPI char *php_replace_controlchars_ex(char *str, size_t len)
static void php_replace_controlchars(char *str, size_t len)
{
unsigned char *s = (unsigned char *)str;
unsigned char *e = (unsigned char *)str + len;

if (!str) {
return (NULL);
}
ZEND_ASSERT(str != NULL);

while (s < e) {

if (iscntrl(*s)) {
*s='_';
}
s++;
}

return (str);
}
/* }}} */

PHPAPI char *php_replace_controlchars(char *str)
{
return php_replace_controlchars_ex(str, strlen(str));
}

PHPAPI php_url *php_url_parse(char const *str)
Expand Down Expand Up @@ -133,7 +121,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port

if (e + 1 == ue) { /* only scheme is available */
ret->scheme = zend_string_init(s, (e - s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->scheme), ZSTR_LEN(ret->scheme));
php_replace_controlchars(ZSTR_VAL(ret->scheme), ZSTR_LEN(ret->scheme));
return ret;
}

Expand All @@ -155,13 +143,13 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
}

ret->scheme = zend_string_init(s, (e-s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->scheme), ZSTR_LEN(ret->scheme));
php_replace_controlchars(ZSTR_VAL(ret->scheme), ZSTR_LEN(ret->scheme));

s = e + 1;
goto just_path;
} else {
ret->scheme = zend_string_init(s, (e-s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->scheme), ZSTR_LEN(ret->scheme));
php_replace_controlchars(ZSTR_VAL(ret->scheme), ZSTR_LEN(ret->scheme));

if (e + 2 < ue && *(e + 2) == '/') {
s = e + 3;
Expand Down Expand Up @@ -227,14 +215,14 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
if ((p = zend_memrchr(s, '@', (e-s)))) {
if ((pp = memchr(s, ':', (p-s)))) {
ret->user = zend_string_init(s, (pp-s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
php_replace_controlchars(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));

pp++;
ret->pass = zend_string_init(pp, (p-pp), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->pass), ZSTR_LEN(ret->pass));
php_replace_controlchars(ZSTR_VAL(ret->pass), ZSTR_LEN(ret->pass));
} else {
ret->user = zend_string_init(s, (p-s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
php_replace_controlchars(ZSTR_VAL(ret->user), ZSTR_LEN(ret->user));
}

s = p + 1;
Expand Down Expand Up @@ -283,7 +271,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
}

ret->host = zend_string_init(s, (p-s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->host), ZSTR_LEN(ret->host));
php_replace_controlchars(ZSTR_VAL(ret->host), ZSTR_LEN(ret->host));

if (e == ue) {
return ret;
Expand All @@ -299,7 +287,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
p++;
if (p < e) {
ret->fragment = zend_string_init(p, (e - p), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->fragment), ZSTR_LEN(ret->fragment));
php_replace_controlchars(ZSTR_VAL(ret->fragment), ZSTR_LEN(ret->fragment));
} else {
ret->fragment = ZSTR_EMPTY_ALLOC();
}
Expand All @@ -311,7 +299,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
p++;
if (p < e) {
ret->query = zend_string_init(p, (e - p), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->query), ZSTR_LEN(ret->query));
php_replace_controlchars(ZSTR_VAL(ret->query), ZSTR_LEN(ret->query));
} else {
ret->query = ZSTR_EMPTY_ALLOC();
}
Expand All @@ -320,7 +308,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port

if (s < e || s == ue) {
ret->path = zend_string_init(s, (e - s), 0);
php_replace_controlchars_ex(ZSTR_VAL(ret->path), ZSTR_LEN(ret->path));
php_replace_controlchars(ZSTR_VAL(ret->path), ZSTR_LEN(ret->path));
}

return ret;
Expand Down
1 change: 0 additions & 1 deletion ext/standard/url.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ PHPAPI size_t php_url_decode(char *str, size_t len); /* return value: length of
PHPAPI size_t php_raw_url_decode(char *str, size_t len); /* return value: length of decoded string */
PHPAPI zend_string *php_url_encode(char const *s, size_t len);
PHPAPI zend_string *php_raw_url_encode(char const *s, size_t len);
PHPAPI char *php_replace_controlchars_ex(char *str, size_t len);

#define PHP_URL_SCHEME 0
#define PHP_URL_HOST 1
Expand Down
30 changes: 15 additions & 15 deletions ext/standard/versioning.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,36 +77,36 @@ php_canonicalize_version(const char *version)

typedef struct {
const char *name;
uint8_t name_len;
int order;
} special_forms_t;

static int
compare_special_version_forms(char *form1, char *form2)
static int compare_special_version_forms(char *form1, char *form2)
{
int found1 = -1, found2 = -1;
special_forms_t special_forms[11] = {
{"dev", 0},
{"alpha", 1},
{"a", 1},
{"beta", 2},
{"b", 2},
{"RC", 3},
{"rc", 3},
{"#", 4},
{"pl", 5},
{"p", 5},
{NULL, 0},
{ZEND_STRL("dev"), 0},
{ZEND_STRL("alpha"), 1},
{ZEND_STRL("a"), 1},
{ZEND_STRL("beta"), 2},
{ZEND_STRL("b"), 2},
{ZEND_STRL("RC"), 3},
{ZEND_STRL("rc"), 3},
{ZEND_STRL("#"), 4},
{ZEND_STRL("pl"), 5},
{ZEND_STRL("p"), 5},
{NULL, 0, 0},
};
special_forms_t *pp;

for (pp = special_forms; pp && pp->name; pp++) {
if (strncmp(form1, pp->name, strlen(pp->name)) == 0) {
if (strncmp(form1, pp->name, pp->name_len) == 0) {
found1 = pp->order;
break;
}
}
for (pp = special_forms; pp && pp->name; pp++) {
if (strncmp(form2, pp->name, strlen(pp->name)) == 0) {
if (strncmp(form2, pp->name, pp->name_len) == 0) {
found2 = pp->order;
break;
}
Expand Down
Loading