Elijah Newren <newren@xxxxxxxxx> writes: >> It makes sense, but check_dup() sounds like its use pattern would be >> >> if (check_dup(it) == NO_DUP) >> add(it); >> >> where it is more like "add it but just once". >> >> By the way, is a strset a set or a bag? If it makes the second add > > strset is a set; there is no way to get duplicate entries. > >> an no-op, perhaps your check_dup() is what strset_add() should do >> itself? What builtin/shortlog.c::check_dup() does smells like it is >> a workaround for the lack of a naturally-expected feature. > > Is the expectation that strset_add() would return a boolean for > whether a new entry was added? It seems to be a reasonable expectation that the caller can tell if the add was "already there and was a no-op", judging from what we saw in the shortlog code, which was the first audience the API was introduced to support. It seems to benefit from it if it were available, and has to work around the lack of it with check_dup() wrapper.