Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:49776
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain wafel.org from 212.29.160.232 cause and error)
Message-ID: <4C984D20.7030304@wafel.org>
Date: Tue, 21 Sep 2010 08:13:52 +0200
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.11pre) Gecko/20100917 Shredder/3.1.5pre
MIME-Version: 1.0
To: Chris Stockton <chrisstocktonaz@gmail.com>
CC: Michael Maclean <mgdm@php.net>, internals@lists.php.net
References: <4C97BA1B.8070601@wafel.org>	<4C97CF70.6080701@php.net> <AANLkTikU_DLubgR+kMYRLnNxw5jxu6PwN-NwrG8_c+mm@mail.gmail.com>
In-Reply-To: <AANLkTikU_DLubgR+kMYRLnNxw5jxu6PwN-NwrG8_c+mm@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] Allowing an array as a parameter for snmpget()/snmpgetnext()
From: jorrit@wafel.org (Jorrit Kronjee)

 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