Could you add few sentences explaining how is it better?
On Fri, Dec 18, 2009 at 03:32:32PM +0300, Alexander Sabourenkov wrote:
> Index: snmpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.25
> diff -u snmpd.h
> --- snmpd.h 6 Jun 2009 18:38:01 -0000 1.25
> +++ snmpd.h 18 Dec 2009 12:10:17 -0000
> @@ -209,9 +209,9 @@
> long long sm_request;
>
> long long sm_error;
> -#define sm_nonrepeaters sm_error
> long long sm_errorindex;
> -#define sm_maxrepetitions sm_errorindex
> + long long sm_nonrepeaters;
> + long long sm_maxrepetitions;
>
> struct ber_element *sm_pdu;
> struct ber_element *sm_pduend;
> Index: snmpe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.24
> diff -u snmpe.c
> @@ -477,19 +479,95 @@
> return (BER_TYPE_OCTETSTRING);
> }
>
> +static struct ber_element *
> +snmpe_br_maxreps(struct snmp_message *msg, struct ber_element *start_elm,
> + struct ber_oid *start_oid, int len)
> +{
> + char buf[BUFSIZ];
> + struct ber_element *a, *b, *c, *d, *first, *last;
> + struct ber_oid oids[SNMPD_MAXVARBINDLEN];
> + int repnum, vb_count, vb_idx = 0;
> +
> + msg->sm_errorindex = len;
> + log_debug("snmpe_br_maxreps: maxreps=%d", msg->sm_maxrepetitions);
> +
> + /* stuff in the first oid we have already */
> + bcopy(start_oid, oids, sizeof(*start_oid));
> + log_debug("snmpe_br_maxreps: varbind %d, oid %s", vb_idx,
> + smi_oidstring(start_oid, buf, sizeof(buf)));
> +
> + vb_idx = 1;
> + a = start_elm;
> +
> + /* build a list of oids to iterate over */
> + while ((a != NULL) && (a->be_type != BER_TYPE_EOC)) {
> + msg->sm_errorindex ++;
> + a = a->be_next;
> +
> + if (a->be_class != BER_CLASS_UNIVERSAL ||
> + a->be_type != BER_TYPE_SEQUENCE)
> + continue;
> + if ((b = a->be_sub) == NULL)
> + continue;
> + if (ber_get_oid(b, &(oids[vb_idx])) != 0)
> + return NULL;
> + if (oids[vb_idx].bo_n < BER_MIN_OID_LEN ||
> + oids[vb_idx].bo_n > BER_MAX_OID_LEN)
> + return NULL;
> +
> + log_debug("snmpe_br_maxreps: varbind %d, oid %s", vb_idx,
> + smi_oidstring(&(oids[vb_idx]), buf, sizeof(buf)));
> + vb_idx ++;
> + }
> + vb_count = vb_idx;
> + log_debug("snmpe_br_maxreps: %d varbinds.", vb_count);
> +
> + msg->sm_errorindex = 0;
> + msg->sm_error = SNMP_ERROR_NONE;
> +
> + first = last = NULL;
> +
> + for (repnum = 0; repnum < msg->sm_maxrepetitions; repnum++) {
> + for(vb_idx=0; vb_idx<vb_count; vb_idx++) {
> + log_debug("snmpe_br_maxreps: varbind %d, repnum %d oid
> %s", vb_idx, repnum,
> + smi_oidstring(&(oids[vb_idx]), buf,
> sizeof(buf)));
> +
> + if (oids[vb_idx].bo_n == 0)
> + continue;
> + c = ber_add_sequence(NULL);
> + d = mps_getnextreq(c, &(oids[vb_idx]));
> + if (d == NULL) {
> + oids[vb_idx].bo_n = 0;
> + continue;
> + }
> + len += ber_calc_len(c);
> + if (len > SNMPD_MAXVARBINDLEN) {
> + log_debug("len > SNMPD_MAXVARBINDLEN: %d>%d",
> len, SNMPD_MAXVARBINDLEN);
> + ber_free_elements(c);
> + return first;
> + }
> + if (first == NULL)
> + first = c;
> + else
> + last->be_next = c;
> + last = c;
> + }
> + }
> + return first;
> +}
> +
> int
> snmpe_parse(struct sockaddr_storage *ss,
> struct ber_element *root, struct snmp_message *msg)
> {
> struct snmp_stats *stats = &env->sc_stats;
> - struct ber_element *a, *b, *c, *d, *e, *f, *next, *last;
> + struct ber_element *a, *b, *c, *d, *next, *last;
> const char *errstr = "invalid message";
> long long ver, req;
> unsigned long type, errval, erridx;
> - u_int class, state, i = 0, j = 0;
> + u_int class, state, i = 0;
> char *comn, buf[BUFSIZ], host[MAXHOSTNAMELEN];
> struct ber_oid o;
> - size_t len;
>
> bzero(msg, sizeof(*msg));
>
> @@ -591,6 +671,8 @@
> msg->sm_request = req;
> msg->sm_error = errval;
> msg->sm_errorindex = erridx;
> + msg->sm_nonrepeaters = msg->sm_error;
> + msg->sm_maxrepetitions = msg->sm_errorindex;
>
> print_host(ss, host, sizeof(host));
> log_debug("snmpe_parse: %s: SNMPv%d '%s' context %d request %lld",
> @@ -599,7 +681,7 @@
>
> errstr = "invalid varbind element";
> for (i = 1, a = msg->sm_varbind, last = NULL;
> - a != NULL && i < SNMPD_MAXVARBIND; a = next, i++) {
> + a != NULL && i < SNMPD_MAXVARBINDLEN; a = next, i++) {
> next = a->be_next;
>
> if (a->be_class != BER_CLASS_UNIVERSAL ||
> @@ -647,30 +729,28 @@
> msg->sm_error = SNMP_ERROR_READONLY;
> goto varfail;
> case SNMP_C_GETBULKREQ:
> - j = msg->sm_maxrepetitions;
> + msg->sm_error = 0;
> msg->sm_errorindex = 0;
> - msg->sm_error = SNMP_ERROR_NOSUCHNAME;
> - for (d = NULL, len = 0; j > 0; j--) {
> - e = ber_add_sequence(NULL);
> - if (c == NULL)
> - c = e;
> - f = mps_getnextreq(e, &o);
> - if (f == NULL) {
> - ber_free_elements(e);
> - if (d == NULL)
> - goto varfail;
> + if (msg->sm_nonrepeaters) {
> + log_debug("handling %dth
> nonrepeater %s",
> + msg->sm_nonrepeaters,
> smi_oidstring(&o, buf, sizeof(buf)));
> + msg->sm_nonrepeaters--;
> + c = ber_add_sequence(NULL);
> + if ((d = mps_getnextreq(c, &o))
> != NULL)
> break;
> - }
> - len += ber_calc_len(e);
> - if (len > SNMPD_MAXVARBINDLEN) {
> - ber_free_elements(e);
> - break;
> - }
> - if (d != NULL)
> - ber_link_elements(d, e);
> - d = e;
> + ber_free_elements(c);
> + c = NULL;
> + break; /* ignore error */
> }
> - msg->sm_error = 0;
> + if (msg->sm_maxrepetitions == 0) {
> + log_debug("error: no nonreps,
> no maxreps, oid not expected: %s",
> + smi_oidstring(&o, buf,
> sizeof(buf)));
> + msg->sm_error =
> SNMP_ERROR_GENERR;
> + goto varfail;
> + }
> + c = snmpe_br_maxreps(msg, a, &o, i);
> + state = 42; /* break inner loop */
> + i = state * SNMPD_MAXVARBINDLEN; /*
> break outer loop */
> break;
> default:
> goto varfail;
