Skip to content

ext/dba: Minor refactorings to save memory allocations #16222

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

Merged
merged 2 commits into from
Oct 5, 2024
Merged

ext/dba: Minor refactorings to save memory allocations #16222

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
104dedd
ext/dba: Use zend_string for resource key instead of char*
Girgias Oct 4, 2024
e56b9b9
ext/dba: php_dba_make_key() only returns NULL when an exception occurs
Girgias Oct 4, 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
67 changes: 31 additions & 36 deletions ext/dba/dba.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,7 @@ static void php_dba_update(INTERNAL_FUNCTION_PARAMETERS, int mode)
if (key_ht) {
key_str = php_dba_make_key(key_ht);
if (!key_str) {
// TODO ValueError?
RETURN_FALSE;
RETURN_THROWS();
}
}

Expand Down Expand Up @@ -569,36 +568,33 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
RETURN_THROWS();
}

char *resource_key;
size_t resource_key_len = spprintf(&resource_key, 0,
zend_string *resource_key = zend_strpprintf(0,
"dba_%d_%s_%s_%s", persistent, ZSTR_VAL(path), ZSTR_VAL(mode), handler_str ? ZSTR_VAL(handler_str) : ""
);

if (persistent) {
zend_resource *le;

/* try to find if we already have this link in our persistent list */
if ((le = zend_hash_str_find_ptr(&EG(persistent_list), resource_key, resource_key_len)) != NULL) {
if ((le = zend_hash_find_ptr(&EG(persistent_list), resource_key)) != NULL) {
if (le->type != le_pdb) {
// TODO This should never happen
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_FALSE;
}

object_init_ex(return_value, dba_connection_ce);
dba_connection *connection = Z_DBA_CONNECTION_P(return_value);
connection->info = (dba_info *)le->ptr;
connection->hash = zend_string_init(resource_key, resource_key_len, persistent);
if (persistent) {
GC_MAKE_PERSISTENT_LOCAL(connection->hash);
}
connection->hash = zend_string_dup(resource_key, /* persistent */ true);
GC_MAKE_PERSISTENT_LOCAL(connection->hash);

if (zend_hash_exists(&DBA_G(connections), connection->hash)) {
zend_hash_del(&DBA_G(connections), connection->hash);
}

zend_hash_add_new(&DBA_G(connections), connection->hash, return_value);
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
return;
}
}
Expand All @@ -607,7 +603,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
hptr = DBA_G(default_hptr);
if (!hptr) {
php_error_docref(NULL, E_WARNING, "No default handler selected");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_FALSE;
}
ZEND_ASSERT(hptr->name);
Expand All @@ -617,7 +613,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)

if (!hptr->name) {
php_error_docref(NULL, E_WARNING, "Handler \"%s\" is not available", ZSTR_VAL(handler_str));
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_FALSE;
}
}
Expand All @@ -641,13 +637,13 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)

if (ZSTR_LEN(mode) > 3) {
zend_argument_value_error(2, "must be at most 3 characters");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_THROWS();
}
if (ZSTR_LEN(mode) == 3) {
if (ZSTR_VAL(mode)[2] != 't') {
zend_argument_value_error(2, "third character must be \"t\"");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_THROWS();
}
is_test_lock = true;
Expand All @@ -660,7 +656,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
case '-':
if ((hptr->flags & DBA_LOCK_ALL) == 0) {
php_error_docref(NULL, E_WARNING, "Locking cannot be disabled for handler %s", hptr->name);
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_FALSE;
}
is_lock_ignored = true;
Expand All @@ -682,7 +678,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
break;
default:
zend_argument_value_error(2, "second character must be one of \"d\", \"l\", \"-\", or \"t\"");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_THROWS();
}
} else {
Expand Down Expand Up @@ -751,7 +747,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
break;
default:
zend_argument_value_error(2, "first character must be one of \"r\", \"w\", \"c\", or \"n\"");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_THROWS();
}
if (!lock_file_mode) {
Expand All @@ -760,17 +756,17 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
if (is_test_lock) {
if (is_lock_ignored) {
zend_argument_value_error(2, "cannot combine mode \"-\" (no lock) and \"t\" (test lock)");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_THROWS();
}
if (!lock_mode) {
if ((hptr->flags & DBA_LOCK_ALL) == 0) {
php_error_docref(NULL, E_WARNING, "Handler %s uses its own locking which doesn't support mode modifier t (test lock)", hptr->name);
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_FALSE;
} else {
php_error_docref(NULL, E_WARNING, "Handler %s doesn't uses locking for this mode which makes modifier t (test lock) obsolete", hptr->name);
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
RETURN_FALSE;
}
} else {
Expand All @@ -780,7 +776,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)

zval *connection_zval;
dba_connection *connection;
if ((connection_zval = zend_hash_str_find(&DBA_G(connections), resource_key, resource_key_len)) == NULL) {
if ((connection_zval = zend_hash_find(&DBA_G(connections), resource_key)) == NULL) {
object_init_ex(return_value, dba_connection_ce);
connection = Z_DBA_CONNECTION_P(return_value);

Expand All @@ -792,9 +788,11 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
connection->info->driver_flags = driver_flags;
connection->info->flags = (hptr->flags & ~DBA_LOCK_ALL) | (lock_flag & DBA_LOCK_ALL) | (persistent ? DBA_PERSISTENT : 0);
connection->info->lock.mode = lock_mode;
connection->hash = zend_string_init(resource_key, resource_key_len, persistent);
if (persistent) {
connection->hash = zend_string_dup(resource_key, /* persistent */ true);
GC_MAKE_PERSISTENT_LOCAL(connection->hash);
} else {
connection->hash = zend_string_copy(resource_key);
}
} else {
ZVAL_COPY(return_value, connection_zval);
Expand Down Expand Up @@ -860,7 +858,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
}
if (!connection->info->lock.fp) {
/* stream operation already wrote an error message */
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
zval_ptr_dtor(return_value);
RETURN_FALSE;
}
Expand All @@ -881,7 +879,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
}
if (!connection->info->fp) {
/* stream operation already wrote an error message */
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
zval_ptr_dtor(return_value);
RETURN_FALSE;
}
Expand All @@ -891,7 +889,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
*/
if (SUCCESS != php_stream_cast(connection->info->fp, PHP_STREAM_AS_FD, (void*)&connection->info->fd, 1)) {
php_error_docref(NULL, E_WARNING, "Could not cast stream");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
zval_ptr_dtor(return_value);
RETURN_FALSE;
#ifdef F_SETFL
Expand Down Expand Up @@ -927,24 +925,24 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent)
php_error_docref(NULL, E_WARNING, "Driver initialization failed for handler: %s", hptr->name);
}
}
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
zval_ptr_dtor(return_value);
RETURN_FALSE;
}

connection->info->hnd = hptr;

if (persistent) {
if (zend_register_persistent_resource(resource_key, resource_key_len, connection->info, le_pdb) == NULL) {
if (zend_register_persistent_resource_ex(connection->hash, connection->info, le_pdb) == NULL) {
php_error_docref(NULL, E_WARNING, "Could not register persistent resource");
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
zval_ptr_dtor(return_value);
RETURN_FALSE;
}
}

zend_hash_add_new(&DBA_G(connections), connection->hash, return_value);
efree(resource_key);
zend_string_release_ex(resource_key, /* persistent */ false);
}
/* }}} */

Expand Down Expand Up @@ -1004,8 +1002,7 @@ PHP_FUNCTION(dba_exists)
if (key_ht) {
key_str = php_dba_make_key(key_ht);
if (!key_str) {
// TODO ValueError?
RETURN_FALSE;
RETURN_THROWS();
}
}

Expand Down Expand Up @@ -1051,8 +1048,7 @@ PHP_FUNCTION(dba_fetch)
if (key_ht) {
key_str = php_dba_make_key(key_ht);
if (!key_str) {
// TODO ValueError?
RETURN_FALSE;
RETURN_THROWS();
}
}

Expand Down Expand Up @@ -1185,8 +1181,7 @@ PHP_FUNCTION(dba_delete)
if (key_ht) {
key_str = php_dba_make_key(key_ht);
if (!key_str) {
// TODO ValueError?
RETURN_FALSE;
RETURN_THROWS();
}
}

Expand Down
Loading