[rrd-developers] Preview for updated RRDTool PHP extension and userland wrapper

[Dan Cech]

Benny Baumann wrote:
>> - remove requirement to send argument count to rrd_graph
> Reworked argument parsing and preparation; function much more clean now
>> - add support for rrd_xport
> Some rework here too
>> - remove requirement to send argument count to rrd_fetch
> Reworked argument parsing and preparation; function much more clean now
>> - support multiple update values in rrd_update
> Reworked argument parsing and preparation; function much more clean now.

I like the work you did moving out the args array creation, good idea.

I'm confused about the purpose of setting args[0] = "dummy", is there a
reason behind this or could we simplify things by removing it?

Regarding the argument checks, I don't see any compelling reason to
break compatibility with old code which passes the argument count as the
3rd parameter.

This is why my patch separated the argument count check from the
zend_parse_parameters call, otherwise the result was the
WRONG_PARAM_COUNT error.  For newly-added functions there is obviously
no need to support any old usage.

Basically my thinking is that there should be no reason that this new
version won't be able to work with existing php code built against the
old version, which will make things much easier from a maintenance
perspective in that users don't have to care whether upgrading to the
new version of the extension will break existing php code.

Those who are better versed in how this will affect getting the new
version packaged for debian, etc can probably give more insight on this
point.

> Now it uses the internal array copy routine or builds the necessary
> structure for strings. Makes source more readable and reduces branches.

Looks fine to me.

>> - support rrd_lastupdate
> Didn't compile for me. Had to change for rrd_lastupdate_r to make it
> work (perhaps an error in rrd.h???)

Hmm, was working for me.  No matter using rrd_lastupdate_r is neater.

>> - support rrd_first
> Reworked argument parsing and preparation
>> - support rrd_info
> Reworked argument parsing and preparation

Looks like we're making some headway!

Dan