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