Hello Greg,

Wednesday, February 11, 2009, 6:07:27 AM, you wrote:

> Hi,

> While tracking down a problem in one of phar's tests, I found what might 
> be a problem in RecursiveDirectoryIterator's handling of flags.  Here is 
> a crude patch demonstrating the issue, and wondering if this is 
> something to be concerned about.  Basically, we're mixing long and int, 
> which could lead to truncation in unpredictable ways.

> Greg

> Index: spl_directory.c
> ===================================================================
> RCS file: /repository/php-src/ext/spl/spl_directory.c,v
> retrieving revision 1.45.2.27.2.23.2.40
> diff -u -r1.45.2.27.2.23.2.40 spl_directory.c
> --- spl_directory.c    31 Dec 2008 11:15:43 -0000    1.45.2.27.2.23.2.40
> +++ spl_directory.c    15 Feb 2009 21:45:00 -0000
> @@ -215,7 +215,7 @@
>  /* open a directory resource */
>  static void spl_filesystem_dir_open(spl_filesystem_object* intern, char 
> *path TSRMLS_DC)
>  {
> -    int skip_dots = intern->flags & SPL_FILE_DIR_SKIPDOTS;
> +    int skip_dots = (intern->flags & SPL_FILE_DIR_SKIPDOTS) ? 1 : 0;

While I wouldn't mind this part, I don't see a reason for it (and it
generates slower code, though any file access of course is a million times
slower anyway). Either way, we only do non zero checks, so that it doesn't
matter whether the result is one of 0 and 1 or 0 and something non zero.

If there is a compiler warning, then that can only come from flags being
long and we would need to switch from 'int skip_dots' to 'long skip_dots'.
>  
>      intern->type = SPL_FS_DIR;
>      intern->_path_len = strlen(path);
> @@ -314,7 +314,7 @@
>      case SPL_FS_DIR:
>          spl_filesystem_dir_open(intern, source->_path TSRMLS_CC);
>          /* read until we hit the position in which we were before */
> -        skip_dots = source->flags & SPL_FILE_DIR_SKIPDOTS;
> +        skip_dots = (source->flags & SPL_FILE_DIR_SKIPDOTS) ? 1 : 0;
>          for(index = 0; index < source->u.dir.index; ++index) {
>              do {
>                  spl_filesystem_dir_read(intern TSRMLS_CC);
> @@ -600,7 +600,7 @@
>  #define DIT_CTOR_FLAGS  0x00000001
>  #define DIT_CTOR_GLOB   0x00000002
>  
> -void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, int 
> ctor_flags) /* {{{ */
> +void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, long 
> ctor_flags) /* {{{ */

Sounds fine.

>  {
>      spl_filesystem_object *intern;
>      char *path;
> @@ -698,7 +698,7 @@
>  SPL_METHOD(DirectoryIterator, next)
>  {
>      spl_filesystem_object *intern = 
> (spl_filesystem_object*)zend_object_store_get_object(getThis() TSRMLS_CC);
> -    int skip_dots = intern->flags & SPL_FILE_DIR_SKIPDOTS;
> +    int skip_dots = (intern->flags & SPL_FILE_DIR_SKIPDOTS) ? 1 : 0;
>  
>      intern->u.dir.index++;
>      do {





Best regards,
 Marcus


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to