Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: more const for attr code
To:
tech@openbsd.org
Date:
Tue, 12 May 2026 11:10:41 +0200

Download raw body.

Thread
attr_optadd() should use a const void * for the data passed in.
In the end the data is copied so it makes sense and it would all be cool
with the exception of attr_lookup(). Passing a const void * triggers a bit
of a catch-22. Making the data in struct attr const makes free() unhappy
and not doing it makes the lookup problematic.  The only solution is to
move to CH_LOCATE so the needle and comparison function can be made const
clean.

Making this const correct can be useful for regress testing this code.
-- 
:wq Claudio

Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.347 rde.h
--- rde.h	7 May 2026 11:21:24 -0000	1.347
+++ rde.h	7 May 2026 18:40:50 -0000
@@ -451,7 +451,7 @@ int		 attr_writebuf(struct ibuf *, uint8
 		    uint16_t);
 void		 attr_init(void);
 int		 attr_optadd(struct rde_aspath *, uint8_t, uint8_t,
-		    void *, uint16_t);
+		    const void *, uint16_t);
 struct attr	*attr_optget(const struct rde_aspath *, uint8_t);
 void		 attr_copy(struct rde_aspath *, const struct rde_aspath *);
 int		 attr_equal(const struct rde_aspath *,
Index: rde_attr.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_attr.c,v
diff -u -p -r1.144 rde_attr.c
--- rde_attr.c	7 May 2026 20:35:19 -0000	1.144
+++ rde_attr.c	9 May 2026 20:46:56 -0000
@@ -57,8 +57,10 @@ attr_writebuf(struct ibuf *buf, uint8_t 
 }
 
 /* optional attribute specific functions */
-static struct attr *attr_alloc(uint8_t, uint8_t, void *, uint16_t, uint64_t);
-static struct attr *attr_lookup(uint8_t, uint8_t, void *, uint16_t, uint64_t);
+static struct attr *attr_alloc(uint8_t, uint8_t, const void *, uint16_t,
+    uint64_t);
+static struct attr *attr_lookup(uint8_t, uint8_t, const void *, uint16_t,
+    uint64_t);
 static void attr_put(struct attr *);
 
 static SIPHASH_KEY	 attrkey;
@@ -93,7 +95,7 @@ attr_init(void)
 
 int
 attr_optadd(struct rde_aspath *asp, uint8_t flags, uint8_t type,
-    void *data, uint16_t len)
+    const void *data, uint16_t len)
 {
 	unsigned int	 l;
 	struct attr	*a, *t;
@@ -255,7 +257,8 @@ attr_freeall(struct rde_aspath *asp)
 }
 
 struct attr *
-attr_alloc(uint8_t flags, uint8_t type, void *data, uint16_t len, uint64_t hash)
+attr_alloc(uint8_t flags, uint8_t type, const void *data, uint16_t len,
+    uint64_t hash)
 {
 	struct attr	*a;
 
@@ -286,11 +289,33 @@ attr_alloc(uint8_t flags, uint8_t type, 
 	return (a);
 }
 
+struct lookup_attr {
+	const u_char	*data;
+	uint16_t	 len;
+	uint8_t		 flags;
+	uint8_t		 type;
+};
+
+static inline int
+attr_cmp(const void *va, void *vb)
+{
+	const struct attr *oa = va;
+	struct lookup_attr *ob = vb;
+
+	if (oa->type != ob->type)
+		return 0;
+	if (oa->flags != ob->flags)
+		return 0;
+	if (oa->len != ob->len)
+		return 0;
+	return (oa->len == 0 || memcmp(oa->data, ob->data, oa->len) == 0);
+}
+
 struct attr *
-attr_lookup(uint8_t flags, uint8_t type, void *data, uint16_t len,
+attr_lookup(uint8_t flags, uint8_t type, const void *data, uint16_t len,
     uint64_t hash)
 {
-	struct attr		needle;
+	struct lookup_attr	needle;
 
 	flags &= ~ATTR_DEFMASK;	/* normalize mask */
 
@@ -298,9 +323,8 @@ attr_lookup(uint8_t flags, uint8_t type,
 	needle.type = type;
 	needle.len = len;
 	needle.data = data;
-	needle.hash = hash;
 
-	return CH_FIND(attr_tree, &attrtable, &needle);
+	return CH_LOCATE(attr_tree, &attrtable, hash, attr_cmp, &needle);
 }
 
 void