Implemented Feature #60524 (sys_temp_dir) #262

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants

Added a new configuration directive which allows it to change the
temporary directory, the default behavior is unchanged.

This is a useful option if you use all/some hosts inside of one .ini file
with sections and want to change the temp dir per user (maybe it's not
allowed to write outside the users home directory). Since the TMPDIR
variable affects the whole php that way can not be used for this scenario.

(see https://bugs.php.net/bug.php?id=60524)

php.ini-development
@@ -731,6 +731,10 @@ user_dir =
; On windows:
; extension_dir = "ext"
+; Directory where the temporary files should be placed.
+; Defaults to the system defaut (see sys_get_temp_dir)
@Majkl578

Majkl578 Jan 18, 2013

Contributor

typo in "default"

php.ini-production
@@ -732,6 +732,10 @@ user_dir =
; On windows:
; extension_dir = "ext"
+; Directory where the temporary files should be placed.
+; Defaults to the system defaut (see sys_get_temp_dir)
@Majkl578

Majkl578 Jan 18, 2013

Contributor

same here

main/php_open_temporary_file.c
+ /* Is there a temporary directory "system_tmp_dir" in .ini defined? */
+ {
+ char *system_tmp_dir = PG(system_tmp_dir);
+ if(system_tmp_dir){
@lstrojny

lstrojny Jan 19, 2013

Contributor

There is a space missing

main/php_open_temporary_file.c
+ if(system_tmp_dir){
+ int len = strlen(system_tmp_dir);
+ if (len >= 2 && system_tmp_dir[len - 1] == DEFAULT_SLASH) {
+ temporary_directory = zend_strndup(system_tmp_dir, len - 1);
@lstrojny

lstrojny Jan 19, 2013

Contributor

Where do you clean that up?

@alexkazik

alexkazik Jan 19, 2013

Sorry, what should I clean up? (the variable temporary_directory will be already freed in php_shutdown_temporary_directory, same file)

@lstrojny

lstrojny Jan 19, 2013

Contributor

Ah, sorry. Than it’s fine.

Contributor

lstrojny commented Jan 19, 2013

Nice patch. See my few remarks and we need a test for it. Could you add one?

would you like a test like this?
tests/basic/req60524.phpt:
--TEST--
Req #60524 (Specify temporary directory)
--INI--
system_tmp_dir=/path/to/temp/dir
--FILE--

--EXPECT--
/path/to/temp/dir

(if yes, update my branch, right? - I'm new too this stuff)

Contributor

lstrojny commented Jan 19, 2013

Yep, looks good.

main/php_open_temporary_file.c
+ {
+ char *system_tmp_dir = PG(system_tmp_dir);
+ if (system_tmp_dir) {
+ int len = strlen(system_tmp_dir);
@lstrojny

lstrojny Jan 19, 2013

Contributor

Declarations need to be on top of the block so that our Windows build doesn’t fail

@laruence

laruence Jan 19, 2013

Owner

it's okey, it at the beginning of a block

@lstrojny

lstrojny Jan 19, 2013

Contributor

Ah, @laruence is right. Stupid me.

Implemented Feature #60524 (sys_temp_dir)
Added a new configuration directive which allows it to change the
temporary directory, the default behavior is unchanged.

This is a useful option if you use all/some hosts inside of one .ini file
with sections and want to change the temp dir per user (maybe it's not
allowed to write outside the users home directory). Since the TMPDIR
variable affects the whole php that way can not be used for this scenario.

(see https://bugs.php.net/bug.php?id=60524)

Comment on behalf of stas at php.net:

merged

@php-pulls php-pulls closed this Jan 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment