Merge pull request #4740 from opensourcerouting/omgwtfbbq

lib: OMGWTFBBQ × 2
FRRouting · Jul 29, 2019 · a6ffcbd · a6ffcbd
2 parents 506fc1a + 4937287
commit a6ffcbd
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 25 deletions.
diff --git a/lib/compiler.h b/lib/compiler.h
@@ -165,6 +165,13 @@ extern "C" {
 		_min_a < _min_b ? _min_a : _min_b;                             \
 	})
 
+#define numcmp(a, b)                                                           \
+	({                                                                     \
+		typeof(a) _cmp_a = (a);                                        \
+		typeof(b) _cmp_b = (b);                                        \
+		(_cmp_a < _cmp_b) ? -1 : ((_cmp_a > _cmp_b) ? 1 : 0);          \
+	})
+
 #ifndef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE,MEMBER)

diff --git a/lib/prefix.c b/lib/prefix.c
@@ -628,8 +628,15 @@ int prefix_match_network_statement(const struct prefix *n,
 	return 1;
 }
 
-void prefix_copy(struct prefix *dest, const struct prefix *src)
+#ifdef __clang_analyzer__
+#undef prefix_copy	/* cf. prefix.h */
+#endif
+
+void prefix_copy(union prefixptr udest, union prefixconstptr usrc)
 {
+	struct prefix *dest = udest.p;
+	const struct prefix *src = usrc.p;
+
 	dest->family = src->family;
 	dest->prefixlen = src->prefixlen;
 
@@ -674,8 +681,11 @@ void prefix_copy(struct prefix *dest, const struct prefix *src)
  * the same.  Note that this routine has the same return value sense
  * as '==' (which is different from prefix_cmp).
  */
-int prefix_same(const struct prefix *p1, const struct prefix *p2)
+int prefix_same(union prefixconstptr up1, union prefixconstptr up2)
 {
+	const struct prefix *p1 = up1.p;
+	const struct prefix *p2 = up2.p;
+
 	if ((p1 && !p2) || (!p1 && p2))
 		return 0;
 
@@ -712,57 +722,59 @@ int prefix_same(const struct prefix *p1, const struct prefix *p2)
 }
 
 /*
- * Return 0 if the network prefixes represented by the struct prefix
- * arguments are the same prefix, and 1 otherwise.  Network prefixes
- * are considered the same if the prefix lengths are equal and the
- * network parts are the same.  Host bits (which are considered masked
+ * Return -1/0/1 comparing the prefixes in a way that gives a full/linear
+ * order.
+ *
+ * Network prefixes are considered the same if the prefix lengths are equal
+ * and the network parts are the same.  Host bits (which are considered masked
  * by the prefix length) are not significant.  Thus, 10.0.0.1/8 and
  * 10.0.0.2/8 are considered equivalent by this routine.  Note that
  * this routine has the same return sense as strcmp (which is different
  * from prefix_same).
  */
-int prefix_cmp(const struct prefix *p1, const struct prefix *p2)
+int prefix_cmp(union prefixconstptr up1, union prefixconstptr up2)
 {
+	const struct prefix *p1 = up1.p;
+	const struct prefix *p2 = up2.p;
 	int offset;
 	int shift;
+	int i;
 
 	/* Set both prefix's head pointer. */
 	const uint8_t *pp1;
 	const uint8_t *pp2;
 
 	if (p1->family != p2->family)
-		return 1;
+		return numcmp(p1->family, p2->family);
 	if (p1->family == AF_FLOWSPEC) {
 		pp1 = (const uint8_t *)p1->u.prefix_flowspec.ptr;
 		pp2 = (const uint8_t *)p2->u.prefix_flowspec.ptr;
 
 		if (p1->u.prefix_flowspec.prefixlen !=
 		    p2->u.prefix_flowspec.prefixlen)
-			return 1;
+			return numcmp(p1->u.prefix_flowspec.prefixlen,
+				      p2->u.prefix_flowspec.prefixlen);
 
 		offset = p1->u.prefix_flowspec.prefixlen;
 		while (offset--)
 			if (pp1[offset] != pp2[offset])
-				return 1;
+				return numcmp(pp1[offset], pp2[offset]);
 		return 0;
 	}
 	pp1 = p1->u.val;
 	pp2 = p2->u.val;
 
 	if (p1->prefixlen != p2->prefixlen)
-		return 1;
+		return numcmp(p1->prefixlen, p2->prefixlen);
 	offset = p1->prefixlen / PNBBY;
 	shift = p1->prefixlen % PNBBY;
 
-	if (shift)
-		if (maskbit[shift] & (pp1[offset] ^ pp2[offset]))
-			return 1;
-
-	while (offset--)
-		if (pp1[offset] != pp2[offset])
-			return 1;
+	i = memcmp(pp1, pp2, offset);
+	if (i)
+		return i;
 
-	return 0;
+	return numcmp(pp1[offset] & maskbit[shift],
+		      pp2[offset] & maskbit[shift]);
 }
 
 /*

diff --git a/lib/prefix.h b/lib/prefix.h
@@ -410,12 +410,20 @@ extern const char *prefix2str(union prefixconstptr, char *, int);
 extern int prefix_match(const struct prefix *, const struct prefix *);
 extern int prefix_match_network_statement(const struct prefix *,
 					  const struct prefix *);
-extern int prefix_same(const struct prefix *, const struct prefix *);
-extern int prefix_cmp(const struct prefix *, const struct prefix *);
+extern int prefix_same(union prefixconstptr, union prefixconstptr);
+extern int prefix_cmp(union prefixconstptr, union prefixconstptr);
 extern int prefix_common_bits(const struct prefix *, const struct prefix *);
-extern void prefix_copy(struct prefix *dest, const struct prefix *src);
+extern void prefix_copy(union prefixptr, union prefixconstptr);
 extern void apply_mask(struct prefix *);
 
+#ifdef __clang_analyzer__
+/* clang-SA doesn't understand transparent unions, making it think that the
+ * target of prefix_copy is uninitialized.  So just memset the target.
+ * cf. https://bugs.llvm.org/show_bug.cgi?id=42811
+ */
+#define prefix_copy(a, b) ({ memset(a, 0, sizeof(*a)); prefix_copy(a, b); })
+#endif
+
 extern struct prefix *sockunion2prefix(const union sockunion *dest,
 				       const union sockunion *mask);
 extern struct prefix *sockunion2hostprefix(const union sockunion *,

diff --git a/lib/typesafe.h b/lib/typesafe.h
@@ -434,7 +434,7 @@ macro_inline type *prefix ## _find_gteq(struct prefix##_head *h,               \
 	struct ssort_item *sitem = h->sh.first;                                \
 	int cmpval = 0;                                                        \
 	while (sitem && (cmpval = cmpfn_nuq(                                   \
-			container_of(sitem, type, field.si), item) < 0))       \
+			container_of(sitem, type, field.si), item)) < 0)       \
 		sitem = sitem->next;                                           \
 	return container_of_null(sitem, type, field.si);                       \
 }                                                                              \
@@ -444,7 +444,7 @@ macro_inline type *prefix ## _find_lt(struct prefix##_head *h,                 \
 	struct ssort_item *prev = NULL, *sitem = h->sh.first;                  \
 	int cmpval = 0;                                                        \
 	while (sitem && (cmpval = cmpfn_nuq(                                   \
-			container_of(sitem, type, field.si), item) < 0))       \
+			container_of(sitem, type, field.si), item)) < 0)       \
 		sitem = (prev = sitem)->next;                                  \
 	return container_of_null(prev, type, field.si);                        \
 }                                                                              \
@@ -499,7 +499,7 @@ macro_inline type *prefix ## _find(struct prefix##_head *h, const type *item)  \
 	struct ssort_item *sitem = h->sh.first;                                \
 	int cmpval = 0;                                                        \
 	while (sitem && (cmpval = cmpfn(                                       \
-			container_of(sitem, type, field.si), item) < 0))       \
+			container_of(sitem, type, field.si), item)) < 0)       \
 		sitem = sitem->next;                                           \
 	if (!sitem || cmpval > 0)                                              \
 		return NULL;                                                   \