From 3c372901bd1255a1a7b1ffce551e2b33e09e0a96 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 29 Jun 2022 16:13:35 +0100 Subject: [PATCH] Add support to pass driver flags to DBA handlers Currently only LMDB with DBA_LMDB_USE_SUB_DIR/DBA_LMDB_NO_SUB_DIR are supported --- NEWS | 1 + UPGRADING | 7 ++- ext/dba/dba.c | 23 ++++++++-- ext/dba/dba.stub.php | 16 +++++-- ext/dba/dba_arginfo.h | 13 +++++- ext/dba/dba_lmdb.c | 18 +++++++- ext/dba/php_dba.h | 4 ++ ext/dba/php_lmdb.h | 1 + ext/dba/tests/dba_flags_arg.phpt | 14 ++++++ ext/dba/tests/dba_lmdb_flags.phpt | 76 +++++++++++++++++++++++++++++++ 10 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 ext/dba/tests/dba_flags_arg.phpt create mode 100644 ext/dba/tests/dba_lmdb_flags.phpt diff --git a/NEWS b/NEWS index ecde7c963a6..e13f36ce622 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,7 @@ PHP NEWS - DBA: . Fixed LMDB driver memory leak on DB creation failure (Girgias) + . Fixed GH-8856 (dba: lmdb: allow to override the MDB_NOSUBDIR flag). (Girgias) - Random: . Fixed bug GH-9067 (random extension is not thread safe). (cmb) diff --git a/UPGRADING b/UPGRADING index 8d5dd2c0b9a..85717cba606 100644 --- a/UPGRADING +++ b/UPGRADING @@ -88,6 +88,11 @@ PHP 8.2 UPGRADE NOTES . Exposed multiple new constants from libcurl 7.62 to 7.80. . Added new function curl_upkeep() to perform any connection upkeep checks. +- DBA: + . The LMDB Driver now accepts the DBA_LMDB_USE_SUB_DIR or DBA_LMDB_NO_SUB_DIR + flags to determine if it should create a sub directory or not when creating + a database file. + - OCI8: . Added an oci8.prefetch_lob_size directive and oci_set_prefetch_lob() function to tune LOB query performance by reducing the number of @@ -183,7 +188,7 @@ PHP 8.2 UPGRADE NOTES - DBA . dba_open() and dba_popen() now have the following enforced function signature - dba_open(string $path, string $mode, ?string $handler = null, int $permission = 0o644, int $map_size = 0) + dba_open(string $path, string $mode, ?string $handler = null, int $permission = 0o644, int $map_size = 0, ?int $flags = null) . dba_fetch()'s optional skip argument is now at the end in line with PHP userland semantics its signature now is: dba_fetch(string|array $key, $dba, int $skip = 0): string|false diff --git a/ext/dba/dba.c b/ext/dba/dba.c index 3ba946e64a0..bf0e9c5add1 100644 --- a/ext/dba/dba.c +++ b/ext/dba/dba.c @@ -359,6 +359,7 @@ PHP_MINIT_FUNCTION(dba) REGISTER_INI_ENTRIES(); le_db = zend_register_list_destructors_ex(dba_close_rsrc, NULL, "dba", module_number); le_pdb = zend_register_list_destructors_ex(dba_close_pe_rsrc, dba_close_rsrc, "dba persistent", module_number); + register_dba_symbols(module_number); return SUCCESS; } /* }}} */ @@ -478,10 +479,12 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent) zend_string *handler_str = NULL; zend_long permission = 0644; zend_long map_size = 0; + zend_long driver_flags = DBA_DEFAULT_DRIVER_FLAGS; + bool is_flags_null = true; zend_string *persistent_resource_key = NULL; - if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "PS|S!ll", &path, &mode, &handler_str, - &permission, &map_size)) { + if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "PS|S!lll!", &path, &mode, &handler_str, + &permission, &map_size, &driver_flags, &is_flags_null)) { RETURN_THROWS(); } @@ -503,6 +506,11 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent) RETURN_THROWS(); } + if (!is_flags_null && driver_flags < 0) { + zend_argument_value_error(6, "must be greater or equal than 0"); + RETURN_THROWS(); + } + if (persistent) { zend_resource *le; @@ -720,6 +728,7 @@ static void php_dba_open(INTERNAL_FUNCTION_PARAMETERS, bool persistent) info->mode = modenr; info->file_permission = permission; info->map_size = map_size; + info->driver_flags = driver_flags; info->flags = (hptr->flags & ~DBA_LOCK_ALL) | (lock_flag & DBA_LOCK_ALL) | (persistent ? DBA_PERSISTENT : 0); info->lock.mode = lock_mode; @@ -835,9 +844,15 @@ restart: } } - if (error || hptr->open(info, &error) != SUCCESS) { + if (error || hptr->open(info, &error) == FAILURE) { dba_close(info); - php_error_docref(NULL, E_WARNING, "Driver initialization failed for handler: %s%s%s", hptr->name, error?": ":"", error?error:""); + if (EXPECTED(!EG(exception))) { + if (error) { + php_error_docref(NULL, E_WARNING, "Driver initialization failed for handler: %s: %s", hptr->name, error); + } else { + php_error_docref(NULL, E_WARNING, "Driver initialization failed for handler: %s", hptr->name); + } + } FREE_PERSISTENT_RESOURCE_KEY(); RETURN_FALSE; } diff --git a/ext/dba/dba.stub.php b/ext/dba/dba.stub.php index 7066f0f772e..ece850f437b 100644 --- a/ext/dba/dba.stub.php +++ b/ext/dba/dba.stub.php @@ -2,11 +2,21 @@ /** @generate-class-entries */ -/** @return resource|false */ -function dba_popen(string $path, string $mode, ?string $handler = null, int $permission = 0o644, int $map_size = 0) {} +#ifdef DBA_LMDB +/** @var int */ +const DBA_LMDB_USE_SUB_DIR = 0; +/** + * @var int + * @cvalue MDB_NOSUBDIR + */ +const DBA_LMDB_NO_SUB_DIR = UNKNOWN; +#endif /** @return resource|false */ -function dba_open(string $path, string $mode, ?string $handler = null, int $permission = 0o644, int $map_size = 0) {} +function dba_popen(string $path, string $mode, ?string $handler = null, int $permission = 0o644, int $map_size = 0, ?int $flags = null) {} + +/** @return resource|false */ +function dba_open(string $path, string $mode, ?string $handler = null, int $permission = 0o644, int $map_size = 0, ?int $flags = null) {} /** @param resource $dba */ function dba_close($dba): void {} diff --git a/ext/dba/dba_arginfo.h b/ext/dba/dba_arginfo.h index 769d2b382c6..63f845e192b 100644 --- a/ext/dba/dba_arginfo.h +++ b/ext/dba/dba_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 1957dd08c4efcfa765bd15c8d9ae9e69edec5db5 */ + * Stub hash: 1a02eaf9da45edb40720620e3beef43fd19dd520 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_dba_popen, 0, 0, 2) ZEND_ARG_TYPE_INFO(0, path, IS_STRING, 0) @@ -7,6 +7,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_dba_popen, 0, 0, 2) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, handler, IS_STRING, 1, "null") ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, permission, IS_LONG, 0, "0644") ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, map_size, IS_LONG, 0, "0") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, flags, IS_LONG, 1, "null") ZEND_END_ARG_INFO() #define arginfo_dba_open arginfo_dba_popen @@ -95,3 +96,13 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(dba_list, arginfo_dba_list) ZEND_FE_END }; + +static void register_dba_symbols(int module_number) +{ +#if defined(DBA_LMDB) + REGISTER_LONG_CONSTANT("DBA_LMDB_USE_SUB_DIR", 0, CONST_CS | CONST_PERSISTENT); +#endif +#if defined(DBA_LMDB) + REGISTER_LONG_CONSTANT("DBA_LMDB_NO_SUB_DIR", MDB_NOSUBDIR, CONST_CS | CONST_PERSISTENT); +#endif +} diff --git a/ext/dba/dba_lmdb.c b/ext/dba/dba_lmdb.c index 75f9827a610..16454533245 100644 --- a/ext/dba/dba_lmdb.c +++ b/ext/dba/dba_lmdb.c @@ -40,12 +40,28 @@ DBA_OPEN_FUNC(lmdb) { MDB_env *env; MDB_txn *txn; - int rc, flags = MDB_NOSUBDIR; + int rc; int mode = info->file_permission; zend_long map_size = info->map_size; ZEND_ASSERT(map_size >= 0); + /* By default use the MDB_NOSUBDIR flag */ + int flags = MDB_NOSUBDIR; + /* Use flags passed by the user for driver flags */ + if (info->driver_flags != DBA_DEFAULT_DRIVER_FLAGS) { + ZEND_ASSERT(info->driver_flags >= 0); + switch (info->driver_flags) { + case 0: + case MDB_NOSUBDIR: + flags = info->driver_flags; + break; + default: + zend_argument_value_error(6, "must be either DBA_LMDB_USE_SUB_DIR or DBA_LMDB_NO_SUB_DIR for LMDB driver"); + return FAILURE; + } + } + /* Add readonly flag if DB is opened in read only mode */ if (info->mode == DBA_READER) { flags |= MDB_RDONLY; diff --git a/ext/dba/php_dba.h b/ext/dba/php_dba.h index 28ca887222f..ce59e4851d9 100644 --- a/ext/dba/php_dba.h +++ b/ext/dba/php_dba.h @@ -44,12 +44,16 @@ typedef struct dba_info { int fd; int file_permission; zend_long map_size; + /* -1 for default driver flags */ + zend_long driver_flags; /* private */ int flags; /* whether and how dba did locking and other flags*/ struct dba_handler *hnd; dba_lock lock; } dba_info; +#define DBA_DEFAULT_DRIVER_FLAGS -1 + #define DBA_LOCK_READER (0x0001) #define DBA_LOCK_WRITER (0x0002) #define DBA_LOCK_CREAT (0x0004) diff --git a/ext/dba/php_lmdb.h b/ext/dba/php_lmdb.h index 421f6a00949..3f19e88b3d8 100644 --- a/ext/dba/php_lmdb.h +++ b/ext/dba/php_lmdb.h @@ -4,6 +4,7 @@ #ifdef DBA_LMDB #include "php_dba.h" +#include DBA_FUNCS(lmdb); diff --git a/ext/dba/tests/dba_flags_arg.phpt b/ext/dba/tests/dba_flags_arg.phpt new file mode 100644 index 00000000000..d9d72831bc1 --- /dev/null +++ b/ext/dba/tests/dba_flags_arg.phpt @@ -0,0 +1,14 @@ +--TEST-- +DBA new flags ValueError test +--EXTENSIONS-- +dba +--FILE-- +getMessage(), \PHP_EOL; +} +?> +--EXPECT-- +dba_open(): Argument #6 ($flags) must be greater or equal than 0 diff --git a/ext/dba/tests/dba_lmdb_flags.phpt b/ext/dba/tests/dba_lmdb_flags.phpt new file mode 100644 index 00000000000..d0cd2de2072 --- /dev/null +++ b/ext/dba/tests/dba_lmdb_flags.phpt @@ -0,0 +1,76 @@ +--TEST-- +DBA LMDB handler flags test +--EXTENSIONS-- +dba +--SKIPIF-- + +--FILE-- +getMessage(), \PHP_EOL; +} + +// Use current test folder +$db_filename = __DIR__; +$db_file = dba_open($db_filename, 'c', $handler, flags: DBA_LMDB_USE_SUB_DIR); +assert($db_file !== false); + +// Check insertion of data +dba_insert("key1", "Content String 1", $db_file); +dba_insert("key2", "Content String 2", $db_file); +dba_insert("key3", "Third Content String", $db_file); +dba_insert("key4", "Another Content String", $db_file); +dba_insert("key5", "The last content string", $db_file); + +// Remove some data +dba_delete("key3", $db_file); +dba_delete("key1", $db_file); + +// Fetch data +$key = dba_firstkey($db_file); +$total_keys = 0; +while ($key) { + echo $key, ': ', dba_fetch($key, $db_file), \PHP_EOL; + $key = dba_nextkey($db_file); + $total_keys++; +} +echo 'Total keys: ', $total_keys, \PHP_EOL; +for ($i = 1; $i < 6; $i++) { + echo "Key $i exists? ", dba_exists("key$i", $db_file) ? "Y" : "N", \PHP_EOL; +} + +// Replace second key data +dba_replace("key2", "Content 2 replaced", $db_file); +echo dba_fetch("key2", $db_file), \PHP_EOL; + +// Close handler +dba_close($db_file); + +?> +--CLEAN-- + +--EXPECT-- +dba_open(): Argument #6 ($flags) must be either DBA_LMDB_USE_SUB_DIR or DBA_LMDB_NO_SUB_DIR for LMDB driver +key2: Content String 2 +key4: Another Content String +key5: The last content string +Total keys: 3 +Key 1 exists? N +Key 2 exists? Y +Key 3 exists? N +Key 4 exists? Y +Key 5 exists? Y +Content 2 replaced