Re: [PATCH 2/9] commit-graph: stop using signed integers to count bloom filters

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> I wrote these counters in 312cff5207 (bloom: split 'get_bloom_filter()'
>> in two, 2020-09-16) and 59f0d5073f (bloom: encode out-of-bounds filters
>> as non-empty, 2020-09-17), and I don't see a compelling reason that
>> these should be unsigned.
>
> I think that is going backwards though: the question to ask is why
> should these be signed if they cannot ever be negative?

Earlier I gave an example of allowing for a "not yet counted"
sentinel value for a variable or a structure member.  Another
example may be for a function that counts that also needs to signal
an error, and as usual in any C programs, the natural way to do so
for any function whose "normal" return values are non-negative
integers is to signal errors with a negative value.

Note that a structure member or a variable that does not need such a
"not yet counted" sentinel value (e.g., it may have a separate
"counted already" member associated with it, or the nature of the
thing it counts does not have such "not yet counted" state), and it
is possible for such a variable to live happily with a function that
can signal an error.

It means the variable that receives the counted result from such a
function may be able to use only half a range of values as its type
implies, if that helper function is the only source of information
that is assigned to it, though.

If the counter in question never needs to store such a sentinel
value itself, then I am OK for it to be unsigned, and that is
exactly why I said "not always a valid excuse".  But if the counter
variable or structure member has to work with functions that need to
return sentinel values (like platform natural int that can use the
usual "negative is an error, non-negative is a normal result"), it
may have less chance to trigger the -Wsign-compare irritation, if
you made it also signed.