1172013 - NetworkManager causes snmp OID not increasing

commit 664ed943f63dfe9393e959840ecd23c31c9d8f89
Author: Bill Fenner <fenner@gmail.com>
Date:   Wed Aug 27 16:02:57 2014 -0400

    Handle duplicates in a binary_array container
    
    The CONTAINER_KEY_ALLOW_DUPLICATES setting is fundamentally flawed;
    it really effectively meant "I promise I won't insert duplicates
    so don't check at insert time".  However, the ip-forward-mib
    sets this flag but relies on the duplicate prevention at insert
    time under certain scenarios (e.g., multiple attachments to the
    same subnet on MacOS), resulting in a loop in ip-forward-mib
    in these scenarios.  So, now it means "check for duplicates at
    getnext time" - the binary search will find an arbitrary one
    of the entries with the same key, and when we've incremented
    we have to check whether or not we've actually incremented past
    any duplicates.  This costs an extra key compare per getnext.
    
    If there's a scenario in the future where a MIB implementation
    can really guarantee that it isn't inserting duplicates, we
    might want to add a "CONTAINER_KEY_I_PROMISE_I_WONT_INSERT_DUPLICATES",
    that disables the insertion check but doesn't perform the getnext
    check.

diff --git a/snmplib/container_binary_array.c b/snmplib/container_binary_array.c
index 249a3a9..10ae67f 100644
--- a/snmplib/container_binary_array.c
+++ b/snmplib/container_binary_array.c
@@ -284,6 +284,22 @@ netsnmp_binary_array_get(netsnmp_container *c, const void *key, int exact)
     if (key) {
         if ((index = binary_search(key, c, exact)) == -1)
             return NULL;
+        if (!exact &&
+            c->flags & CONTAINER_KEY_ALLOW_DUPLICATES) {
+            int result;
+
+            /*
+             * If duplicates are allowed, we have to be extra
+             * sure that we didn't just increment to a duplicate,
+             * thus causing a getnext loop.
+             */
+            result = c->compare(t->data[index], key);
+            while (result == 0) {
+                if (++index == t->count)
+                   return NULL;
+                result = c->compare(t->data[index], key);
+            }
+        }
     }
 
     return t->data[index];
diff --git a/testing/fulltests/unit-tests/T021binary_array_oid_duplicates_clib.c b/testing/fulltests/unit-tests/T021binary_array_oid_duplicates_clib.c
new file mode 100644
index 0000000..c027329
--- /dev/null
+++ b/testing/fulltests/unit-tests/T021binary_array_oid_duplicates_clib.c
@@ -0,0 +1,72 @@
+/* HEADER Testing duplicate handling in binary OID array */
+
+/* Much copied from T012 */
+static const char test_name[] = "binary-array-of-OIDs-duplicate-test";
+oid o1 = 1;
+oid o2 = 2;
+oid o3 = 6;
+oid o4 = 8;
+oid o5 = 9;
+oid ox = 7;
+oid oy = 10;
+netsnmp_index i1, i2, i3, i4, i5, ix, iy, *ip;
+netsnmp_index *b[] = { &i4, &i2, &i3, &i1, &i5 };
+netsnmp_container *c;
+int i;
+
+init_snmp(test_name);
+
+c = netsnmp_container_get_binary_array();
+c->compare = netsnmp_compare_netsnmp_index;
+netsnmp_binary_array_options_set(c, 1,
+				 CONTAINER_KEY_ALLOW_DUPLICATES);
+
+i1.oids = &o1;
+i2.oids = &o2;
+i3.oids = &o3;
+i4.oids = &o4;
+i5.oids = &o5;
+ix.oids = &ox;
+iy.oids = &oy;
+i1.len = i2.len = i3.len = i4.len = i5.len = ix.len = iy.len = 1;
+
+for (i = 0; i < sizeof(b)/sizeof(b[0]); ++i)
+    CONTAINER_INSERT(c, b[i]);
+
+#define MAX_ROUNDS 6
+/* Insert some duplicates of i4; also insert a duplicate of
+ * i1 to move the contents of the array around. */
+for (i = 0; i < MAX_ROUNDS; ++i) {
+    switch (i) {
+	case 0:
+	    /* First round: no insert */
+	    break;
+	case 1:
+	case 2:
+	case 4:
+	case 5:
+	    /* Insert another duplicate of our target object */
+	    CONTAINER_INSERT(c, &i4);
+	    break;
+	case 3:
+	    /* Insert a dulicate of an earlier OID, so that it
+	     * changes the binary search behavior */
+	    CONTAINER_INSERT(c, &i1);
+	    break;
+    }
+    /* Primary requirement: getnext returns the next value! */
+    ip = CONTAINER_FIND(c, &i4);
+    OKF(ip, ("FIND returned a value"));
+    OKF(c->compare(&i4, ip) == 0,
+        ("FIND returned oid %" NETSNMP_PRIo "d", ip->oids[0]));
+    ip = CONTAINER_NEXT(c, &i4);
+    OKF(ip, ("NEXT returned a value"));
+    OKF(c->compare(&i5, ip) == 0,
+        ("NEXT returned index 5 = %" NETSNMP_PRIo "d", i5.oids[0]));
+}
+
+while ((ip = CONTAINER_FIRST(c)))
+  CONTAINER_REMOVE(c, ip);
+CONTAINER_FREE(c);
+
+snmp_shutdown(test_name);