ext/standard/proc_open.c: Minor refactorings (php#18085)

Add const modifier
Use unsigned types when the source type is unsigned
Use HashTable instead of zvals where possible

Author: Girgias

ext/standard/proc_open.c

@@ -136,11 +136,11 @@ int openpty(int *master, int *slave, char *name, struct termios *termp, struct w
 
 static int le_proc_open; /* Resource number for `proc` resources */
 
-/* {{{ _php_array_to_envp
+/* {{{ php_array_to_envp
  * Process the `environment` argument to `proc_open`
  * Convert into data structures which can be passed to underlying OS APIs like `exec` on POSIX or
  * `CreateProcessW` on Win32 */
-static php_process_env _php_array_to_envp(zval *environment)
+ZEND_ATTRIBUTE_NONNULL static php_process_env php_array_to_envp(const HashTable *environment)
 {
 	zval *element;
 	php_process_env env;
@@ -154,13 +154,9 @@ static php_process_env _php_array_to_envp(zval *environment)
 
 	memset(&env, 0, sizeof(env));
 
-	if (!environment) {
-		return env;
-	}
-
-	uint32_t cnt = zend_hash_num_elements(Z_ARRVAL_P(environment));
+	uint32_t cnt = zend_hash_num_elements(environment);
 
-	if (cnt < 1) {
+	if (cnt == 0) {
 #ifndef PHP_WIN32
 		env.envarray = (char **) ecalloc(1, sizeof(char *));
 #endif
@@ -172,7 +168,7 @@ static php_process_env _php_array_to_envp(zval *environment)
 	zend_hash_init(env_hash, cnt, NULL, NULL, 0);
 
 	/* first, we have to get the size of all the elements in the hash */
-	ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(environment), key, element) {
+	ZEND_HASH_FOREACH_STR_KEY_VAL(environment, key, element) {
 		str = zval_get_string(element);
 
 		if (ZSTR_LEN(str) == 0) {
@@ -221,7 +217,7 @@ static php_process_env _php_array_to_envp(zval *environment)
 /* }}} */
 
 /* {{{ _php_free_envp
- * Free the structures allocated by `_php_array_to_envp` */
+ * Free the structures allocated by php_array_to_envp */
 static void _php_free_envp(php_process_env env)
 {
 #ifndef PHP_WIN32
@@ -506,7 +502,7 @@ typedef struct _descriptorspec_item {
 	int mode_flags;                  /* mode for opening FDs: r/o, r/w, binary (on Win32), etc */
 } descriptorspec_item;
 
-static zend_string *get_valid_arg_string(zval *zv, int elem_num) {
+static zend_string *get_valid_arg_string(zval *zv, uint32_t elem_num) {
 	zend_string *str = zval_get_string(zv);
 	if (!str) {
 		return NULL;
@@ -518,7 +514,7 @@ static zend_string *get_valid_arg_string(zval *zv, int elem_num) {
 		return NULL;
 	}
 
-	if (strlen(ZSTR_VAL(str)) != ZSTR_LEN(str)) {
+	if (zend_str_has_nul_byte(str)) {
 		zend_value_error("Command array element %d contains a null byte", elem_num);
 		zend_string_release(str);
 		return NULL;
@@ -630,7 +626,7 @@ static zend_string *create_win_command_from_args(HashTable *args)
 	zval *arg_zv;
 	bool is_prog_name = true;
 	bool is_cmd_execution = false;
-	int elem_num = 0;
+	uint32_t elem_num = 0;
 
 	ZEND_HASH_FOREACH_VAL(args, arg_zv) {
 		zend_string *arg_str = get_valid_arg_string(arg_zv, ++elem_num);
@@ -778,11 +774,11 @@ static zend_result convert_command_to_use_shell(wchar_t **cmdw, size_t cmdw_len)
 
 #ifndef PHP_WIN32
 /* Convert command parameter array passed as first argument to `proc_open` into command string */
-static zend_string* get_command_from_array(HashTable *array, char ***argv, int num_elems)
+static zend_string* get_command_from_array(const HashTable *array, char ***argv, uint32_t num_elems)
 {
 	zval *arg_zv;
 	zend_string *command = NULL;
-	int i = 0;
+	uint32_t i = 0;
 
 	*argv = safe_emalloc(sizeof(char *), num_elems + 1, 0);
 
@@ -810,16 +806,16 @@ static zend_string* get_command_from_array(HashTable *array, char ***argv, int n
 }
 #endif
 
-static descriptorspec_item* alloc_descriptor_array(HashTable *descriptorspec)
+static descriptorspec_item* alloc_descriptor_array(const HashTable *descriptorspec)
 {
 	uint32_t ndescriptors = zend_hash_num_elements(descriptorspec);
 	return ecalloc(ndescriptors, sizeof(descriptorspec_item));
 }
 
-static zend_string* get_string_parameter(zval *array, int index, char *param_name)
+static zend_string* get_string_parameter(const HashTable *ht, unsigned int index, const char *param_name)
 {
 	zval *array_item;
-	if ((array_item = zend_hash_index_find(Z_ARRVAL_P(array), index)) == NULL) {
+	if ((array_item = zend_hash_index_find(ht, index)) == NULL) {
 		zend_value_error("Missing %s", param_name);
 		return NULL;
 	}
@@ -995,7 +991,7 @@ static zend_result dup_proc_descriptor(php_file_descriptor_t from, php_file_desc
 }
 
 static zend_result redirect_proc_descriptor(descriptorspec_item *desc, int target,
-	descriptorspec_item *descriptors, int ndesc, int nindex)
+	const descriptorspec_item *descriptors, int ndesc, int nindex)
 {
 	php_file_descriptor_t redirect_to = PHP_INVALID_FD;
 
@@ -1030,9 +1026,9 @@ static zend_result redirect_proc_descriptor(descriptorspec_item *desc, int targe
 }
 
 /* Process one item from `$descriptorspec` argument to `proc_open` */
-static zend_result set_proc_descriptor_from_array(zval *descitem, descriptorspec_item *descriptors,
+static zend_result set_proc_descriptor_from_array(const HashTable *ht, descriptorspec_item *descriptors,
 	int ndesc, int nindex, int *pty_master_fd, int *pty_slave_fd) {
-	zend_string *ztype = get_string_parameter(descitem, 0, "handle qualifier");
+	zend_string *ztype = get_string_parameter(ht, 0, "handle qualifier");
 	if (!ztype) {
 		return FAILURE;
 	}
@@ -1042,7 +1038,7 @@ static zend_result set_proc_descriptor_from_array(zval *descitem, descriptorspec
 
 	if (zend_string_equals_literal(ztype, "pipe")) {
 		/* Set descriptor to pipe */
-		zmode = get_string_parameter(descitem, 1, "mode parameter for 'pipe'");
+		zmode = get_string_parameter(ht, 1, "mode parameter for 'pipe'");
 		if (zmode == NULL) {
 			goto finish;
 		}
@@ -1052,16 +1048,16 @@ static zend_result set_proc_descriptor_from_array(zval *descitem, descriptorspec
 		retval = set_proc_descriptor_to_socket(&descriptors[ndesc]);
 	} else if (zend_string_equals(ztype, ZSTR_KNOWN(ZEND_STR_FILE))) {
 		/* Set descriptor to file */
-		if ((zfile = get_string_parameter(descitem, 1, "file name parameter for 'file'")) == NULL) {
+		if ((zfile = get_string_parameter(ht, 1, "file name parameter for 'file'")) == NULL) {
 			goto finish;
 		}
-		if ((zmode = get_string_parameter(descitem, 2, "mode parameter for 'file'")) == NULL) {
+		if ((zmode = get_string_parameter(ht, 2, "mode parameter for 'file'")) == NULL) {
 			goto finish;
 		}
 		retval = set_proc_descriptor_to_file(&descriptors[ndesc], zfile, zmode);
 	} else if (zend_string_equals_literal(ztype, "redirect")) {
 		/* Redirect descriptor to whatever another descriptor is set to */
-		zval *ztarget = zend_hash_index_find_deref(Z_ARRVAL_P(descitem), 1);
+		zval *ztarget = zend_hash_index_find_deref(ht, 1);
 		if (!ztarget) {
 			zend_value_error("Missing redirection target");
 			goto finish;
@@ -1116,7 +1112,7 @@ static zend_result set_proc_descriptor_from_resource(zval *resource, descriptors
 
 #ifndef PHP_WIN32
 #if defined(USE_POSIX_SPAWN)
-static zend_result close_parentends_of_pipes(posix_spawn_file_actions_t * actions, descriptorspec_item *descriptors, int ndesc)
+static zend_result close_parentends_of_pipes(posix_spawn_file_actions_t * actions, const descriptorspec_item *descriptors, int ndesc)
 {
 	int r;
 	for (int i = 0; i < ndesc; i++) {
@@ -1200,9 +1196,10 @@ PHP_FUNCTION(proc_open)
 	HashTable *command_ht;
 	HashTable *descriptorspec; /* Mandatory argument */
 	zval *pipes;               /* Mandatory argument */
-	char *cwd = NULL;                                /* Optional argument */
-	size_t cwd_len = 0;                              /* Optional argument */
-	zval *environment = NULL, *other_options = NULL; /* Optional arguments */
+	char *cwd = NULL;              /* Optional argument */
+	size_t cwd_len = 0;            /* Optional argument */
+	HashTable *environment = NULL; /* Optional arguments */
+	zval *other_options = NULL;    /* Optional arguments */
 
 	php_process_env env;
 	int ndesc = 0;
@@ -1239,7 +1236,7 @@ PHP_FUNCTION(proc_open)
 		Z_PARAM_ZVAL(pipes)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_STRING_OR_NULL(cwd, cwd_len)
-		Z_PARAM_ARRAY_OR_NULL(environment)
+		Z_PARAM_ARRAY_HT_OR_NULL(environment)
 		Z_PARAM_ARRAY_OR_NULL(other_options)
 	ZEND_PARSE_PARAMETERS_END();
 
@@ -1282,7 +1279,7 @@ PHP_FUNCTION(proc_open)
 #endif
 
 	if (environment) {
-		env = _php_array_to_envp(environment);
+		env = php_array_to_envp(environment);
 	}
 
 	descriptors = alloc_descriptor_array(descriptorspec);
@@ -1302,7 +1299,7 @@ PHP_FUNCTION(proc_open)
 				goto exit_fail;
 			}
 		} else if (Z_TYPE_P(descitem) == IS_ARRAY) {
-			if (set_proc_descriptor_from_array(descitem, descriptors, ndesc, (int)nindex,
+			if (set_proc_descriptor_from_array(Z_ARRVAL_P(descitem), descriptors, ndesc, (int)nindex,
 				&pty_master_fd, &pty_slave_fd) == FAILURE) {
 				goto exit_fail;
 			}