Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: bgpd: more const for attr code
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Tue, 12 May 2026 16:39:42 +0200

Download raw body.

Thread
On Tue, May 12, 2026 at 01:37:39PM +0200, Theo Buehler wrote:
> On Tue, May 12, 2026 at 11:10:41AM +0200, Claudio Jeker wrote:
> > 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.
> 
> The idea seems right and the diff correct. I only have a bikeshed
> coloring problem:
> 
> I have missed that the qualifiers in the _cmp function for _ch_locate()
> are asymmetric. This feels very weird to me. Passing non-const to const
> is never a problem.

I did this only to allow the function to pass an argument that can be
modified. Not sure why one would use that. Also it helps to keep the two
pointers apart from each other. Neither are very good reasons but this is
the first user of this API (I had once another one but that bit of code is
not yet ready).
 
> Also, isn't this rather an _eq() function? It returns 1 if the
> arguments are equal 0 if they're distinct: a cmp function does the
> opposite: return 0 if they're equal and != 0 if they're different.
 
Ugh. Now doing this in the chash code is trivial but the attr code
already comes with attr_eq() and attr_equal(). So I went with attr_cmp but
maybe attr_loc_eq() would work and is still kind of short. A bit scared of
people passing the wrong function around.

-- 
:wq Claudio