Re: snmpd(8): clean up variable printing
On Wed, 2022-01-19 at 16:23 +0100, Martijn van Duren wrote:
> The new code uses smi_print_element when debugging is enabled to trace
> calls. Unfortunately the current smi_print_element lacks in quite a few
> departments. This diff rewrites smi_print_element to be more concise
> than what we currently have, without moving into the more complex
> territory that snmp(1) has.
>
> Unknown types are printed in a similar fashion to what tcpdump(8)'s
> snmp output does.
>
> This change helps mostly with exceptions (NOSUCH{OBJECT,INSTANCE} and
> ENDOFMIBVIEW) and distinguishing between different integer types.
>
> I kept the current implementation under smi_print_element_legacy, so
> that we don't change the output of trap handlers. We should probably
> revisit that one at some point, but I don't think to go into that
> territory right now.
>
> OK?
>
> martijn@
>
> p.s. I'm not particularly thrilled about the type hinting, but it's
> the cleanest that I could come up with without being too much of an
> eyesore or filling the screen up even further.
>
ping
Index: smi.c
===
RCS file: /cvs/src/usr.sbin/snmpd/smi.c,v
retrieving revision 1.30
diff -u -p -r1.30 smi.c
--- smi.c 21 Oct 2021 15:08:15 - 1.30
+++ smi.c 29 Jun 2022 11:44:26 -
@@ -46,6 +46,7 @@
#include "snmpd.h"
#include "mib.h"
+#include "application.h"
#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
@@ -461,8 +462,9 @@ smi_debug_elements(struct ber_element *r
}
#endif
+/* Keep around so trap handle scripts don't break */
char *
-smi_print_element(struct ber_element *root)
+smi_print_element_legacy(struct ber_element *root)
{
char*str = NULL, *buf, *p;
size_t len, i;
@@ -520,6 +522,140 @@ smi_print_element(struct ber_element *ro
case BER_TYPE_SET:
default:
str = strdup("");
+ break;
+ }
+
+ return (str);
+
+ fail:
+ free(str);
+ return (NULL);
+}
+
+char *
+smi_print_element(struct ber_element *root)
+{
+ char*str = NULL, *buf, *p;
+ long longv;
+ struct ber_oid o;
+ char strbuf[BUFSIZ];
+
+ switch (root->be_class) {
+ case BER_CLASS_UNIVERSAL:
+ switch (root->be_type) {
+ case BER_TYPE_INTEGER:
+ if (ober_get_integer(root, &v) == -1)
+ goto fail;
+ if (asprintf(&str, "%lld", v) == -1)
+ goto fail;
+ break;
+ case BER_TYPE_OBJECT:
+ if (ober_get_oid(root, &o) == -1)
+ goto fail;
+ if (asprintf(&str, "%s", smi_oid2string(&o, strbuf,
+ sizeof(strbuf), 0)) == -1)
+ goto fail;
+ break;
+ case BER_TYPE_OCTETSTRING:
+ if (ober_get_string(root, &buf) == -1)
+ goto fail;
+ p = reallocarray(NULL, 4, root->be_len + 1);
+ if (p == NULL)
+ goto fail;
+ strvisx(p, buf, root->be_len, VIS_NL);
+ if (asprintf(&str, "\"%s\"", p) == -1) {
+ free(p);
+ goto fail;
+ }
+ free(p);
+ break;
+ case BER_TYPE_NULL:
+ if (asprintf(&str, "null") == -1)
+ goto fail;
+ break;
+ default:
+ /* Should not happen in a valid SNMP packet */
+ if (asprintf(&str, "[U/%u]", root->be_type) == -1)
+ goto fail;
+ break;
+ }
+ break;
+ case BER_CLASS_APPLICATION:
+ switch (root->be_type) {
+ case SNMP_T_IPADDR:
+ if (ober_get_string(root, &buf) == -1)
+ goto fail;
+ if (asprintf(&str, "%s",
+ inet_ntoa(*(struct in_addr *)buf)) == -1)
+ goto fail;
+ break;
+ case SNMP_T_COUNTER32:
+ if (ober_get_integer(root, &v) == -1)
+ goto fail;
+ if (asprintf(&str, "%lld(c32)", v) == -1)
+ goto fail;
+ break;
+ case SNMP_T_GAUGE32:
+ if (ober_get_integer(root, &v) == -1)
+ goto fail;
+ if (asprintf(&str, "%lld(g32)", v) == -1)
+ goto fail;
+
Re: snmpd(8): clean up variable printing
On Wed, 2022-01-19 at 16:23 +0100, Martijn van Duren wrote:
> The new code uses smi_print_element when debugging is enabled to trace
> calls. Unfortunately the current smi_print_element lacks in quite a few
> departments. This diff rewrites smi_print_element to be more concise
> than what we currently have, without moving into the more complex
> territory that snmp(1) has.
>
> Unknown types are printed in a similar fashion to what tcpdump(8)'s
> snmp output does.
>
> This change helps mostly with exceptions (NOSUCH{OBJECT,INSTANCE} and
> ENDOFMIBVIEW) and distinguishing between different integer types.
>
> I kept the current implementation under smi_print_element_legacy, so
> that we don't change the output of trap handlers. We should probably
> revisit that one at some point, but I don't think to go into that
> territory right now.
>
> OK?
>
> martijn@
>
> p.s. I'm not particularly thrilled about the type hinting, but it's
> the cleanest that I could come up with without being too much of an
> eyesore or filling the screen up even further.
Found a missing break after the BER_TYPE_NULL case.
OK?
martijn@
Index: smi.c
===
RCS file: /cvs/src/usr.sbin/snmpd/smi.c,v
retrieving revision 1.30
diff -u -p -r1.30 smi.c
--- smi.c 21 Oct 2021 15:08:15 - 1.30
+++ smi.c 29 Jan 2022 07:28:15 -
@@ -46,6 +46,7 @@
#include "snmpd.h"
#include "mib.h"
+#include "application.h"
#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
@@ -461,8 +462,9 @@ smi_debug_elements(struct ber_element *r
}
#endif
+/* Keep around so trap handle scripts don't break */
char *
-smi_print_element(struct ber_element *root)
+smi_print_element_legacy(struct ber_element *root)
{
char*str = NULL, *buf, *p;
size_t len, i;
@@ -520,6 +522,140 @@ smi_print_element(struct ber_element *ro
case BER_TYPE_SET:
default:
str = strdup("");
+ break;
+ }
+
+ return (str);
+
+ fail:
+ free(str);
+ return (NULL);
+}
+
+char *
+smi_print_element(struct ber_element *root)
+{
+ char*str = NULL, *buf, *p;
+ long longv;
+ struct ber_oid o;
+ char strbuf[BUFSIZ];
+
+ switch (root->be_class) {
+ case BER_CLASS_UNIVERSAL:
+ switch (root->be_type) {
+ case BER_TYPE_INTEGER:
+ if (ober_get_integer(root, &v) == -1)
+ goto fail;
+ if (asprintf(&str, "%lld", v) == -1)
+ goto fail;
+ break;
+ case BER_TYPE_OBJECT:
+ if (ober_get_oid(root, &o) == -1)
+ goto fail;
+ if (asprintf(&str, "%s", smi_oid2string(&o, strbuf,
+ sizeof(strbuf), 0)) == -1)
+ goto fail;
+ break;
+ case BER_TYPE_OCTETSTRING:
+ if (ober_get_string(root, &buf) == -1)
+ goto fail;
+ p = reallocarray(NULL, 4, root->be_len + 1);
+ if (p == NULL)
+ goto fail;
+ strvisx(p, buf, root->be_len, VIS_NL);
+ if (asprintf(&str, "\"%s\"", p) == -1) {
+ free(p);
+ goto fail;
+ }
+ free(p);
+ break;
+ case BER_TYPE_NULL:
+ if (asprintf(&str, "null") == -1)
+ goto fail;
+ break;
+ default:
+ /* Should not happen in a valid SNMP packet */
+ if (asprintf(&str, "[U/%u]", root->be_type) == -1)
+ goto fail;
+ break;
+ }
+ break;
+ case BER_CLASS_APPLICATION:
+ switch (root->be_type) {
+ case SNMP_T_IPADDR:
+ if (ober_get_string(root, &buf) == -1)
+ goto fail;
+ if (asprintf(&str, "%s",
+ inet_ntoa(*(struct in_addr *)buf)) == -1)
+ goto fail;
+ break;
+ case SNMP_T_COUNTER32:
+ if (ober_get_integer(root, &v) == -1)
+ goto fail;
+ if (asprintf(&str, "%lld(c32)", v) == -1)
+ goto fail;
+ break;
+ case SNMP_T_GAUGE32:
+ if (ober_get_integer(root, &v) == -1)
+ goto fail;
+ if (asprintf(&str, "%lld(g32)"
