On Tue Jan 13, 2026 at 8:27 AM CET, Bertrand Drouvot wrote:
> I've probably a too paranoid concern: what if someone use char[N] to store
> binary stuff with embedded null? That would detect it as string and then
> make use of strncmp() and then would provide incorrect result.
>
> While the risk is probably very low, I think it is there.
Added a warning in the comment for these macros. For non of our
usages this was the case (i.e. the char arrays were all storing null
terminated strings). So I'm not too worried about this being a problem
in practice. Especially because in most cases there will be no null byte
in the key, and instead you'll start reading out of bounds, which will
cause problems pretty much immediately during development.
> === 2
>
> The patch introduces foreach_hash() but the foreach_hash_with_hash_value()
> counterpart seems missing. It could be used in TypeCacheTypCallback() and
> InvalidateAttoptCacheCallback().
>
> I think that we could add it or make the new hash_seq_new() accept an extra
> hashvalue parameter? (when set to 0 would call hash_seq_init() and
> hash_seq_init_with_hash_value() otherwise?
I'm not sure it's worth adding that macro. Given there's only two usages
of this function inside our codebase, and I don't expect extensions to
use this low level one. Especially, because to make this macro nice to
use in the two cases that it would apply to we'd have to make it treat 0
as a special value.
> Some minor comments:
fixed all of these.
Finally, I converted the last couple of hash_seq_init stragglers (some
I had missed/were added) and others needed the now newly added
foreach_hash_reset macro to be converted.
