Zend: Refactor virtual_rmdir() to take path lengths

Author: Girgias

UPGRADING.INTERNALS

@@ -107,14 +107,15 @@ PHP 8.4 INTERNALS UPGRADE NOTES
 * Removed ZEND_DIM_ALTERNATIVE_SYNTAX constant. This syntax no longer has a
   specialized error message.
 
-* The virtual_file_ex() function now has an extra parameter for the path
-  length.
-
-* The virtual_chdir_file() function and corresponding VCWD_CHDIR_FILE() macro
-  now have an extra parameter for the path length.
-
-* The virtual_rename() function and corresponding VCWD_RENAME() macro
-  now have extra parameters for the path lengths.
+* The following functions and macros now takes extra arguments for the lengths
+  of paths:
+  - virtual_file_ex()
+  - virtual_chdir_file()
+  - VCWD_CHDIR_FILE()
+  - virtual_rename()
+  - VCWD_RENAME()
+  - virtual_rmdir()
+  - VCWD_RMDIR()
 
 ========================
 2. Build system changes
@@ -394,5 +395,6 @@ PHP 8.4 INTERNALS UPGRADE NOTES
 6. Windows changes
 ========================
 
-* The php_win32_ioutil_rename() function now takes extra arguments for the
-  lengths of the names.
+* The following functions now takes extra arguments for the lengths of paths:
+  - php_win32_ioutil_rmdir()
+  - php_win32_ioutil_rename()

Zend/zend_virtual_cwd.c

@@ -1620,22 +1620,21 @@ CWD_API int virtual_mkdir(const char *pathname, mode_t mode) /* {{{ */
 }
 /* }}} */
 
-CWD_API int virtual_rmdir(const char *pathname) /* {{{ */
+CWD_API zend_result virtual_rmdir(const char *path, size_t path_length) /* {{{ */
 {
 	cwd_state new_state;
-	int retval;
-	size_t path_length = strlen(pathname);
 
 	CWD_STATE_COPY(&new_state, &CWDG(cwd));
-	if (virtual_file_ex(&new_state, pathname, path_length, NULL, CWD_EXPAND)) {
+	if (virtual_file_ex(&new_state, path, path_length, NULL, CWD_EXPAND)) {
 		CWD_STATE_FREE_ERR(&new_state);
-		return -1;
+		return FAILURE;
 	}
 
+	zend_result retval;
 #ifdef ZEND_WIN32
-	retval = php_win32_ioutil_rmdir(new_state.cwd);
+	retval = php_win32_ioutil_rmdir(new_state.cwd, new_state.cwd_length);
 #else
-	retval = rmdir(new_state.cwd);
+	retval = virtual_rmdir_native(new_state.cwd, new_state.cwd_length);
 #endif
 	CWD_STATE_FREE_ERR(&new_state);
 	return retval;

Zend/zend_virtual_cwd.h

@@ -185,7 +185,7 @@ CWD_API int virtual_stat(const char *path, zend_stat_t *buf);
 CWD_API int virtual_lstat(const char *path, zend_stat_t *buf);
 CWD_API int virtual_unlink(const char *path);
 CWD_API int virtual_mkdir(const char *pathname, mode_t mode);
-CWD_API int virtual_rmdir(const char *pathname);
+CWD_API zend_result virtual_rmdir(const char *path, size_t path_len);
 CWD_API DIR *virtual_opendir(const char *pathname);
 CWD_API FILE *virtual_popen(const char *command, const char *type);
 CWD_API int virtual_access(const char *pathname, int mode);
@@ -260,6 +260,10 @@ static zend_always_inline zend_result virtual_rename_native(const char *old_name
 	return (rename(old_name, new_name) == 0) ? SUCCESS : FAILURE;
 }
 
+static zend_always_inline zend_result virtual_rmdir_native(const char *path, ZEND_ATTRIBUTE_UNUSED size_t path_len) {
+	return (rmdir(path) == 0) ? SUCCESS : FAILURE;
+}
+
 #ifdef CWD_EXPORTS
 extern void virtual_cwd_main_cwd_init(uint8_t);
 #endif
@@ -285,8 +289,8 @@ extern void virtual_cwd_main_cwd_init(uint8_t);
 #define VCWD_STAT(path, buff) virtual_stat(path, buff)
 # define VCWD_LSTAT(path, buff) virtual_lstat(path, buff)
 #define VCWD_UNLINK(path) virtual_unlink(path)
-#define VCWD_MKDIR(pathname, mode) virtual_mkdir(pathname, mode)
-#define VCWD_RMDIR(pathname) virtual_rmdir(pathname)
+#define VCWD_MKDIR(path, mode) virtual_mkdir(path, mode)
+#define VCWD_RMDIR(path, path_len) virtual_rmdir(path, path_len)
 #define VCWD_OPENDIR(pathname) virtual_opendir(pathname)
 #define VCWD_POPEN(command, type) virtual_popen(command, type)
 #define VCWD_ACCESS(pathname, mode) virtual_access(pathname, mode)
@@ -311,8 +315,8 @@ extern void virtual_cwd_main_cwd_init(uint8_t);
 #define VCWD_OPEN(path, flags) php_win32_ioutil_open(path, flags)
 #define VCWD_OPEN_MODE(path, flags, mode) php_win32_ioutil_open(path, flags, mode)
 #define VCWD_RENAME(old_name, old_name_length, new_name, new_name_length) php_win32_ioutil_rename(old_name, old_name_length, new_name, new_name_length)
-#define VCWD_MKDIR(pathname, mode) php_win32_ioutil_mkdir(pathname, mode)
-#define VCWD_RMDIR(pathname) php_win32_ioutil_rmdir(pathname)
+#define VCWD_MKDIR(path, mode) php_win32_ioutil_mkdir(path, mode)
+#define VCWD_RMDIR(path, path_length) php_win32_ioutil_rmdir(path, path_length)
 #define VCWD_UNLINK(path) php_win32_ioutil_unlink(path)
 #define VCWD_CHDIR(path) php_win32_ioutil_chdir(path)
 #define VCWD_ACCESS(pathname, mode) tsrm_win32_access(pathname, mode)
@@ -324,7 +328,7 @@ extern void virtual_cwd_main_cwd_init(uint8_t);
 #define VCWD_OPEN_MODE(path, flags, mode)	open(path, flags, mode)
 #define VCWD_RENAME(old_name, old_name_len, new_name, new_name_len) virtual_rename_native(old_name, old_name_len, new_name, new_name_len)
 #define VCWD_MKDIR(path, mode) mkdir(path, mode)
-#define VCWD_RMDIR(pathname) rmdir(pathname)
+#define VCWD_RMDIR(path, path_len) virtual_rmdir_native(path, path_len)
 #define VCWD_UNLINK(path) unlink(path)
 #define VCWD_CHDIR(path) chdir(path)
 #define VCWD_ACCESS(pathname, mode) access(pathname, mode)

main/streams/plain_wrapper.c

@@ -1471,14 +1471,15 @@ static int php_plain_files_rmdir(php_stream_wrapper *wrapper, const char *url, i
 		return 0;
 	}
 
+	size_t url_len = strlen(url);
 #ifdef PHP_WIN32
-	if (!php_win32_check_trailing_space(url, strlen(url))) {
+	if (!php_win32_check_trailing_space(url, url_len)) {
 		php_error_docref1(NULL, url, E_WARNING, "%s", strerror(ENOENT));
 		return 0;
 	}
 #endif
 
-	if (VCWD_RMDIR(url) < 0) {
+	if (VCWD_RMDIR(url, url_len) < 0) {
 		php_error_docref1(NULL, url, E_WARNING, "%s", strerror(errno));
 		return 0;
 	}

win32/ioutil.c

@@ -436,21 +436,6 @@ PW32IO int php_win32_ioutil_unlink_w(const wchar_t *path)
 	return 0;
 }/*}}}*/
 
-PW32IO int php_win32_ioutil_rmdir_w(const wchar_t *path)
-{/*{{{*/
-	int ret = 0;
-
-	PHP_WIN32_IOUTIL_CHECK_PATH_W(path, -1, 0)
-
-	if (!RemoveDirectoryW(path)) {
-		DWORD err = GetLastError();
-		ret = -1;
-		SET_ERRNO_FROM_WIN32_CODE(err);
-	}
-
-	return ret;
-}/*}}}*/
-
 PW32IO int php_win32_ioutil_chdir_w(const wchar_t *path)
 {/*{{{*/
 	int ret = 0;

win32/ioutil.h

@@ -359,30 +359,28 @@ __forceinline static int php_win32_ioutil_unlink(const char *path)
 	return ret;
 }/*}}}*/
 
-__forceinline static int php_win32_ioutil_rmdir(const char *path)
+__forceinline static zend_result php_win32_ioutil_rmdir(const char *path, size_t path_len)
 {/*{{{*/
+	// TODO Is it possible to use php_win32_ioutil_conv_any_to_w() with path_len?
 	PHP_WIN32_IOUTIL_INIT_W(path)
-	int ret = 0;
 	DWORD err = 0;
 
 	if (!pathw) {
 		SET_ERRNO_FROM_WIN32_CODE(ERROR_INVALID_PARAMETER);
-		return -1;
+		return FAILURE;
 	}
 
-	PHP_WIN32_IOUTIL_CHECK_PATH_W(pathw, -1, 1)
+	PHP_WIN32_IOUTIL_CHECK_PATH_W(pathw, FAILURE, 1)
 
+	zend_result ret = SUCCESS;
 	if (!RemoveDirectoryW(pathw)) {
-		err = GetLastError();
-		ret = -1;
+		DWORD err = GetLastError();
+		SET_ERRNO_FROM_WIN32_CODE(err);
+		ret = FAILURE;
 	}
 
 	PHP_WIN32_IOUTIL_CLEANUP_W()
 
-	if (0 > ret) {
-		SET_ERRNO_FROM_WIN32_CODE(err);
-	}
-
 	return ret;
 }/*}}}*/