Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:43071 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 79850 invoked from network); 17 Feb 2009 10:43:38 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 17 Feb 2009 10:43:38 -0000 Authentication-Results: pb1.pair.com header.from=helly@php.net; sender-id=unknown Authentication-Results: pb1.pair.com smtp.mail=helly@php.net; spf=unknown; sender-id=unknown Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.94.56 as permitted sender) X-PHP-List-Original-Sender: helly@php.net X-Host-Fingerprint: 85.214.94.56 aixcept.net Linux 2.6 Received: from [85.214.94.56] ([85.214.94.56:44002] helo=h1149922.serverkompetenz.net) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id 58/4D-33190-7D49A994 for ; Tue, 17 Feb 2009 05:43:38 -0500 Received: from MBOERGER-ZRH.ad.corp.google.com (3-77.107-92.cust.bluewin.ch [92.107.77.3]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by h1149922.serverkompetenz.net (Postfix) with ESMTP id 8C63411FD89; Tue, 17 Feb 2009 11:43:32 +0100 (CET) Date: Tue, 17 Feb 2009 11:40:41 +0100 Reply-To: Marcus Boerger X-Priority: 3 (Normal) Message-ID: <1643750729.20090217114041@marcus-boerger.de> To: Greg Beaver CC: internals@lists.php.net In-Reply-To: <49925D0F.7090601@chiaraquartet.net> References: <49925D0F.7090601@chiaraquartet.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Subject: Re: [PHP-DEV] int/long conflict in spl? From: helly@php.net (Marcus Boerger) 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