--5I6of5zJg18YgZEa
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hello all,



I'm currently in the process of fixing ASN_COUNTER64 OIDs with embedded
Perl agents and I'm sending my patch here in order to have it reviewed.

There were a few problems in the perl/agent/agent.xs code that was
dealing with those types. First, the number is gathered and stored into
an unsigned long variable, which is going to be 32-bits only on most
32-bits architectures. The 64-bits value would thus get truncated to
32-bits. I have fixed this issue using an unsigned long long type,
which we know will be at least 64-bits, and using strtoull() instead of
strtoul() in the case where the number is actually stored into a string
and we need to parse it.

Second, due to the definition of the struct counter64 as two unsigned
long fields, we cannot just use snmp_set_var_typed_value() to set the
value, but need to extract the 32 MSB and LSB by hand and set the fields
ourselves (actually I'm setting those in a temporary struct counter64
variable and then calling snmp_set_var_typed_value() on it). This
requires that we have some notion of the machine's endianness.

There's a last very small change to the code that deals with the
unsigned 32-bits values (ASN_UNSIGNED/ASN_COUNTER/ASN_TIMETICKS): I
replaced the SvIV() macro with SvUV() which is more appropriate, at
least semantically. I am not sure that this fixes a real problem and I
haven't checked, but I believe using SvIV() instead of SvUV() could
cause problems on systems where Perl is compiled without 64-bits ints.

This is working as well as expected, and I can now send ASN_COUNTER64
values within my Perl embedded agent without a problem, but this patch
introduce subtle concerns though. Indeed, the long long type is only
standardized since C99, and was previously just a GCC extension. Given
the definition of the counter64 structure, I guess that either this code
dates back from when long long didn't exist, or that it wasn't an option
for portability reasaons. In case of the latter, it may still not be
appropriate to introduce new code that uses long long types.

The second problem with that patch is that in order to know the
endianness I am using the WORDS_BIGENDIAN macro, because it is the only
related macro I have seen used in the existing code. Using something
such as '#if BYTE_ORDER == LITTLE_ENDIAN ... #else ... #endif' would be
more semantically correct, but I avoided it seeing it was entirely
unused in the source code and that there maybe were good reasons for it.

I'm attaching the patch to this mail.

Cheers,
Maxime

--5I6of5zJg18YgZEa
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="patch-agent.xs"

--- agent.xs Mon Apr 23 11:56:44 2007
+++ /home/mux/agent.xs Mon Apr 23 11:57:22 2007
@@ -767,6 +767,8 @@
u_char *oidbuf = NULL;
size_t ob_len = 0, oo_len = 0;
netsnmp_request_info *request;
+ struct counter64 c64tmp;
+ unsigned long long ulltmp;
u_long utmp;
long ltmp;
oid myoid[MAX_OID_LEN];
@@ -823,12 +825,11 @@

case ASN_UNSIGNED:
case ASN_COUNTER:
- case ASN_COUNTER64:
case ASN_TIMETICKS:
/* We want an integer here */
if ((SvTYPE(value) == SVt_IV) || (SvTYPE(value) == SVt_PVMG)) {
/* Good - got a real one (or a blessed scalar which we have to hope will turn out OK) */
- utmp = SvIV(value);
+ utmp = SvUV(value);
snmp_set_var_typed_value(request->requestvb, (u_char)type,
(u_char *) &utmp, sizeof(utmp));
RETVAL = 1;
@@ -855,6 +856,41 @@
RETVAL = 0;
break;
}
+
+ case ASN_UNSIGNED64:
+ case ASN_COUNTER64:
+ /* We want an integer here */
+ if ((SvTYPE(value) == SVt_IV) || (SvTYPE(value) == SVt_PVMG)) {
+ /* Good - got a real one (or a blessed scalar which we have to hope will turn out OK) */
+ ulltmp = SvUV(value);
+ }
+ else if (SvPOKp(value)) {
+ /* Might be OK - got a string, so try to convert it, allowing base 10, octal, and hex forms */
+ stringptr = SvPV(value, stringlen);
+ ulltmp = strtoull( stringptr, NULL, 0 );
+ if (errno == EINVAL) {
+ snmp_log(LOG_ERR, "Could not convert string to number in setValue: '%s'", stringptr);
+ RETVAL = 0;
+ break;
+ }
+ }
+ else {
+ snmp_log(LOG_ERR, "Non-unsigned-integer value passed to setValue with ASN_UNSIGNED64/ASN_COUNTER64: type was %d\n",
+ SvTYPE(value));
+ RETVAL = 0;
+ break;
+ }
+#ifdef WORDS_BIGENDIAN
+ c64tmp.low = (ulltmp >> 32) & 0xffffffff;
+ c64tmp.high = ulltmp & 0xffffffff;
+#else
+ c64tmp.high = (ulltmp >> 32) & 0xffffffff;
+ c64tmp.low = ulltmp & 0xffffffff;
+#endif
+ snmp_set_var_typed_value(request->requestvb, (u_char)type,
+ (u_char *) &c64tmp, sizeof(c64tmp));
+ RETVAL = 1;
+ break;

case ASN_OCTET_STR:
case ASN_BIT_STR:

--5I6of5zJg18YgZEa
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
--5I6of5zJg18YgZEa
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/...et-snmp-coders

--5I6of5zJg18YgZEa--