Re: com php-src: DOMNodeList elements are accessible through array notation: ext/dom/dom_properties.h ext/dom/nodelist.c ext/dom/php_dom.c
ext/dom/php_dom.h ext/dom/tests/bug67949.phpt

From: Date: Thu, 09 Oct 2014 15:36:23 +0000
Subject: Re: com php-src: DOMNodeList elements are accessible through array notation: ext/dom/dom_properties.h ext/dom/nodelist.c ext/dom/php_dom.c
ext/dom/php_dom.h ext/dom/tests/bug67949.phpt
References: 1 2  Groups: php.cvs 
Request: Send a blank email to php-cvs+get-82219@lists.php.net to get a copy of this message
On 9 Oct 2014, at 17:19, Nikita Popov <nikita.ppv@gmail.com> wrote: > On Sat, Sep 27, 2014 at 1:27 AM, Tjerk Anne Meesters <datibbaw@php.net> wrote: > Commit: 9469db93110b2185f019da0dd66d3537c1a4b7a4 > Author: Florian MARGAINE <florian@margaine.com> Fri, 26 Sep 2014 18:25:56 > +0200 > Parents: 71c39612d5702920dfd198b37254b8bb06454b47 > Branches: PHP-5.6 master > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=9469db93110b2185f019da0dd66d3537c1a4b7a4 > > Log: > DOMNodeList elements are accessible through array notation > > Fixes #67949 > > Bugs: > https://bugs.php.net/67949 > > Changed paths: > M ext/dom/dom_properties.h > M ext/dom/nodelist.c > M ext/dom/php_dom.c > M ext/dom/php_dom.h > A ext/dom/tests/bug67949.phpt > > Some notes: > * The code is not C89 compatible Fixed > * The code directly uses convert_to_long without copying or separation - are you sure > that's safe in this context? Nope, not safe at all :) > * The code does not seem to account for the case where the offset is NULL (i.e. the pointer is > NULL, not the zval having type NULL). Fixed. I’ve now refactored the code so that it calls the ->item() method and ->length property; this is similar to how SPL does things (and there are not many other examples I could use) and would take advantage of classes that extend either DOMNodeList or DOMNamedNodeMap (low likelihood). All in all, there’s less code to maintain, and I think that’s an advantage in itself :D > > Nikita

« previous php.cvs (#82219) next »