[PHP-CVS] cvs: php-src(PHP_5_3) /main php_open_temporary_file.c
iliaa Tue Jun 30 12:20:35 2009 UTC Modified files: (Branch: PHP_5_3) /php-src/main php_open_temporary_file.c Log: MFB: Fixed bug #48465 (sys_get_temp_dir() possibly inconsistent when using TMPDIR). http://cvs.php.net/viewvc.cgi/php-src/main/php_open_temporary_file.c?r1=1.34.2.1.2.10.2.4r2=1.34.2.1.2.10.2.5diff_format=u Index: php-src/main/php_open_temporary_file.c diff -u php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.4 php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.5 --- php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.4Wed Jun 24 20:08:54 2009 +++ php-src/main/php_open_temporary_file.c Tue Jun 30 12:20:35 2009 @@ -16,7 +16,7 @@ +--+ */ -/* $Id: php_open_temporary_file.c,v 1.34.2.1.2.10.2.4 2009/06/24 20:08:54 iliaa Exp $ */ +/* $Id: php_open_temporary_file.c,v 1.34.2.1.2.10.2.5 2009/06/30 12:20:35 iliaa Exp $ */ #include php.h @@ -199,8 +199,15 @@ /* On Unix use the (usual) TMPDIR environment variable. */ { char* s = getenv(TMPDIR); - if (s) { - temporary_directory = strdup(s); + if (s *s) { + int len = strlen(s); + + if (s[len - 1] == DEFAULT_SLASH) { + temporary_directory = zend_strndup(s, len - 1); + } else { + temporary_directory = zend_strndup(s, len); + } + return temporary_directory; } } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-CVS] cvs: php-src(PHP_5_3) /main php_open_temporary_file.c
iliaa Wed Jun 24 12:21:45 2009 UTC Modified files: (Branch: PHP_5_3) /php-src/main php_open_temporary_file.c Log: MFB: Fixed bug #48465 (sys_get_temp_dir() possibly inconsistent when using TMPDIR). http://cvs.php.net/viewvc.cgi/php-src/main/php_open_temporary_file.c?r1=1.34.2.1.2.10.2.2r2=1.34.2.1.2.10.2.3diff_format=u Index: php-src/main/php_open_temporary_file.c diff -u php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.2 php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.3 --- php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.2Wed Dec 31 11:15:47 2008 +++ php-src/main/php_open_temporary_file.c Wed Jun 24 12:21:45 2009 @@ -16,7 +16,7 @@ +--+ */ -/* $Id: php_open_temporary_file.c,v 1.34.2.1.2.10.2.2 2008/12/31 11:15:47 sebastian Exp $ */ +/* $Id: php_open_temporary_file.c,v 1.34.2.1.2.10.2.3 2009/06/24 12:21:45 iliaa Exp $ */ #include php.h @@ -200,7 +200,14 @@ { char* s = getenv(TMPDIR); if (s) { - temporary_directory = strdup(s); + int len = strlen(s); + + if (s[len - 1] == DEFAULT_SLASH) { + temporary_directory = zend_strndup(s, len - 1); + } else { + temporary_directory = zend_strndup(s, len); + } + return temporary_directory; } } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /main php_open_temporary_file.c
Hi Ilia, On Wed, 2009-06-24 at 12:21 +, Ilia Alshanetsky wrote: MFB: Fixed bug #48465 (sys_get_temp_dir() possibly inconsistent when using TMPDIR). First: I'd like to remind about what Lukas wrote as we planned to release 5.3.0 with as little source for trouble as possible: If issues are found/fixed please send the patches to internals for review. Others didn't do that either but at least asked for review on IRC or private mail ... Would be nice if all could follow a stricter review process till 5.3 is tagged (see the mail Lukas will send in a few minutes to internals, too) Seondly: @@ -200,7 +200,14 @@ { char* s = getenv(TMPDIR); if (s) { - temporary_directory = strdup(s); + int len = strlen(s); + + if (s[len - 1] == DEFAULT_SLASH) { + temporary_directory = zend_strndup(s, len - 1); This seems to be broken in case TMPDIR=. johannes + } else { + temporary_directory = zend_strndup(s, len); + } + return temporary_directory; } } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /main php_open_temporary_file.c
Will keep in mind for the future, do you want me to revert the change? Ilia Alshanetsky On 24-Jun-09, at 9:14 AM, Johannes Schlüter wrote: Hi Ilia, On Wed, 2009-06-24 at 12:21 +, Ilia Alshanetsky wrote: MFB: Fixed bug #48465 (sys_get_temp_dir() possibly inconsistent when using TMPDIR). First: I'd like to remind about what Lukas wrote as we planned to release 5.3.0 with as little source for trouble as possible: If issues are found/fixed please send the patches to internals for review. Others didn't do that either but at least asked for review on IRC or private mail ... Would be nice if all could follow a stricter review process till 5.3 is tagged (see the mail Lukas will send in a few minutes to internals, too) Seondly: @@ -200,7 +200,14 @@ { char* s = getenv(TMPDIR); if (s) { - temporary_directory = strdup(s); + int len = strlen(s); + + if (s[len - 1] == DEFAULT_SLASH) { + temporary_directory = zend_strndup(s, len - 1); This seems to be broken in case TMPDIR=. johannes + } else { + temporary_directory = zend_strndup(s, len); + } + return temporary_directory; } } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /main php_open_temporary_file.c
On 24.06.2009, at 18:34, Ilia Alshanetsky wrote: Will keep in mind for the future, do you want me to revert the change? The patch looks safe. However if you would have asked before commiting, Johannes would have said no, since its simply not critical enough to bring any risk to the release process at this point. Then again, this fixes a bug, its in CVS, so it seems like children finger slapping to ask for a revert. We have to trust that people do not intentionally go against what we ask and we have to also hope that people realize that this is a sensitive for the 5_3 branch and then check posts from the RM's before committing. I guess we should maybe define some subject prefix to give such posts additional visible weight and maybe I should more clearly state the current commit policy for 5_3 on some static webpage/wiki. But I also think this is not the right moment to come up with such policies. After we get 5.3.0 stable out expect an RFC that will hopefully outline how to improve things so that the release process is able to produce higher quality code with less hassle for contributors. Back to the patch at hand. Like I said, the patch looks safe, however the devil is in the detail as with many seemlingly simply things. As Johannes has pointed out already, there is the question of what happens if TMPDIR=. So please do revert this patch. regards, Lukas On 24-Jun-09, at 9:14 AM, Johannes Schlüter wrote: Hi Ilia, On Wed, 2009-06-24 at 12:21 +, Ilia Alshanetsky wrote: MFB: Fixed bug #48465 (sys_get_temp_dir() possibly inconsistent when using TMPDIR). First: I'd like to remind about what Lukas wrote as we planned to release 5.3.0 with as little source for trouble as possible: If issues are found/fixed please send the patches to internals for review. Others didn't do that either but at least asked for review on IRC or private mail ... Would be nice if all could follow a stricter review process till 5.3 is tagged (see the mail Lukas will send in a few minutes to internals, too) Seondly: @@ -200,7 +200,14 @@ { char* s = getenv(TMPDIR); if (s) { - temporary_directory = strdup(s); + int len = strlen(s); + + if (s[len - 1] == DEFAULT_SLASH) { + temporary_directory = zend_strndup(s, len - 1); This seems to be broken in case TMPDIR=. johannes + } else { + temporary_directory = zend_strndup(s, len); + } + return temporary_directory; } } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php Lukas Kahwe Smith m...@pooteeweet.org -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-CVS] cvs: php-src(PHP_5_3) /main php_open_temporary_file.c
iliaa Wed Jun 24 20:08:55 2009 UTC Modified files: (Branch: PHP_5_3) /php-src/main php_open_temporary_file.c Log: Revert patch http://cvs.php.net/viewvc.cgi/php-src/main/php_open_temporary_file.c?r1=1.34.2.1.2.10.2.3r2=1.34.2.1.2.10.2.4diff_format=u Index: php-src/main/php_open_temporary_file.c diff -u php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.3 php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.4 --- php-src/main/php_open_temporary_file.c:1.34.2.1.2.10.2.3Wed Jun 24 12:21:45 2009 +++ php-src/main/php_open_temporary_file.c Wed Jun 24 20:08:54 2009 @@ -16,7 +16,7 @@ +--+ */ -/* $Id: php_open_temporary_file.c,v 1.34.2.1.2.10.2.3 2009/06/24 12:21:45 iliaa Exp $ */ +/* $Id: php_open_temporary_file.c,v 1.34.2.1.2.10.2.4 2009/06/24 20:08:54 iliaa Exp $ */ #include php.h @@ -200,14 +200,7 @@ { char* s = getenv(TMPDIR); if (s) { - int len = strlen(s); - - if (s[len - 1] == DEFAULT_SLASH) { - temporary_directory = zend_strndup(s, len - 1); - } else { - temporary_directory = zend_strndup(s, len); - } - + temporary_directory = strdup(s); return temporary_directory; } } -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php