On Tue, May 12, 2026 at 04:39:42PM +0200, Claudio Jeker wrote:
> 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).

I see.

> > 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.

attr_match() might be an option since eq isn't quite right here either.
But the "cmp" really suggests the opposite behavior of what the API wants
and I think we should fix that.

(If you want to land the diff and fix as a follow-up, that's ok with me)