Re: [PATCH 1/1] fsmonitor: don't fill bitmap with entries to be removed

[William Baker <williamtbakeremail@gmail.com>]

On 10/3/19 4:36 PM, Junio C Hamano wrote:

>> +	if (pos >= istate->cache_nr)
>> +		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" >= %"PRIuMAX")",
>> +		    (uintmax_t)pos, (uintmax_t)istate->cache_nr);
> 
> This is how we show size_t values without using "%z" that we avoid,
> but are "pos" and 'cache_nr" size_t or ssize_t?  I thought they are
> plain boring unsigned, so shouldn't we use the plain boring "%u"
> without casting?
> 
> The same comment applies to other uses of uintmax_t cast in this
> patch.
> 

Thanks for catching this.  I will update these BUGs in the next
patch to avoid casting.

>> +# Use test files that start with 'z' so that the entries being added
>> +# and removed appear at the end of the index.
> 
> In other words, future developers are warned against adding entries
> to and leaving them in the index that sort later than z100 in new
> tests they add before this point.  Is the above wording clear enough
> to tell them that, I wonder?
> 

You're understanding is correct, and I agree this comment could be
clearer.  I will fix this up in v2.

Thanks for the feedback!
William