Re: [PATCH] Allowing an array as a parameter for snmpget()/snmpgetnext()

On 09/21/2010 01:36 AM, Chris Stockton wrote: > Hello, > > On Mon, Sep 20, 2010 at 2:17 PM, Michael Maclean <mgdm@php.net> wrote: >> On 20/09/10 20:46, Jorrit Kronjee wrote: >>> Dear list, >>> >>> To better reflect the behavior of the snmpget command that comes with >>> the Net-SNMP package the patch attached makes an array of OIDs as a >>> parameter for the snmpget()/snmpgetnext() function possible. >> <snip> >> >> Looks good to me, for what it's worth. Anyone got objections? >> > I think the patch at a first glance looks good, although it is a minor > cosmetic nitpick, I did catch a line with a couple of extra > semi-colons. > > static void php_snmpv3(INTERNAL_FUNCTION_PARAMETERS, int st) > { > - char *a1, *a2, *a3, *a4, *a5, *a6, *a7, *a8; > - int a1_len, a2_len, a3_len, a4_len, a5_len, a6_len, a7_len, a8_len; > + char *a1, *a2, *a3, *a4, *a5, *a6, *a7, *a8;; > + int a1_len, a2_len, a3_len, a4_len, a5_len, a6_len, a7_len, a8_len;; > > This patch does move around a bit of code, are there some tests to go > along with this patch? > > -Chris Chris, Ouch, I forgot to remove those extra semicolons. Thanks for noticing. What kind of tests are you looking for? I did run a couple of tests against my own SNMP devices to check for regressions, like: oldphp -r 'print_r(snmpget($host, $community, $oid))' newphp -r 'print_r(snmpget($host, $community, $oid))' I also noticed that the tests/ directory for snmp was empty, otherwise I would've run and extended some of those. Shall I just write a couple and include that in a new patch? Regards, Jorrit Kronjee