Hi,
thank you for reviewing my patch.
Antony Dovgal wrote:
Are you sure this part of the patch is correct?
Yes
[[ cut away code ]]
First you free the pdu and then go to the retry label which uses it and
then frees it again on failure.
You misunderstund.
status = snmp_synch_response(ss, pdu, &response);
This line sends 'pdu' on 'ss' (a Net-SNMP session pointer) and sets
'response' to point to a within snmp_sync_response() newly allocated PDU
as a result of the response from the equipment we are communicating with.
I'd say it should look like this:
if (st == SNMP_CMD_GET) {
+ snmp_free_pdu(response);
if ((pdu = snmp_fix_pdu(response, SNMP_MSG_GET))
!= NULL) {
goto retry;
}
No, snmp_free_pdu() is a function that free's memory. Snmp_fix_pdu() is
a function that clones a pdu (response) into a new pdu of a specific
type (SNMP_MSG_GET) while it does that it removes all variables in
error, the return value from that function is a pointer to a new PDU
that will be stored in 'pdu' (or NULL if no variables are left after all
in error has been removed). Obviously you cannot clone memory that you
just free()'d so my original patch is in fact correct.
Could you please test & confirm it?
However, since the (php) snmp module currently do not support
multi-variable-get/set operations we can never recieve an errored
response with more than one variable in it, because of this fact
snmp_fix_pdu() will always return NULL. And the entire codeblock is
dead, that is atleast as far as I understund it.
So, you could say that specific part fixes a possible future problem
that is not applicable today (atleast to my knowledge).
If you're still unsure you may take a look at:
Line 582 snmp_fix_pdu()
http://net-snmp.svn.sourceforge.net/viewvc/net-snmp/trunk/net-snmp/snmplib/snmp_client.c?view=markup
Line 5013 snmp_free_pdu()
http://net-snmp.svn.sourceforge.net/viewvc/net-snmp/trunk/net-snmp/snmplib/snmp_api.c?view=markup
//Gustaf
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
