Re: [PATCH v2 2/2] string-list API: change "nr" and "alloc" to "size_t"

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

Philip Oakley <philipoakley@iee.email> writes:

>> @@ -448,10 +448,10 @@ void shortlog_output(struct shortlog *log)
>>  				(int)UTIL_TO_INT(item), item->string);
>>  		} else {
>>  			struct string_list *onelines = item->util;
>> -			fprintf(log->file, "%s (%d):\n",
>> -				item->string, onelines->nr);
>> -			for (j = onelines->nr - 1; j >= 0; j--) {
>> -				const char *msg = onelines->items[j].string;
>> +			fprintf(log->file, "%s (%"PRIuMAX"):\n",
>> +				item->string, (uintmax_t)onelines->nr);
>> +			for (j = onelines->nr; j >= 1; j--) {
>> +				const char *msg = onelines->items[j - 1].string;
> Does this change of iteration limits and j's offset need a commit
> message comment?

Good eyes.  I also tried to made sure this is the only loop that
counts toward zero that use a variable whose type has become size_t
with this patch, which is unsigned and cannot use "var >= 0" as the
loop termination condition, but what is not in the patch is hard to
know.  If there were loops that used to count down with the .nr or
the .alloc members of string_list structure, they would need to be
updated, but if such a code exists, it is likely a bug already.

I think many internal index string-list.c uses are still "int"
(cf. get_entry_index() and functions on the call graph that leads to
it, like add_entry() and string_list_remove()) that wants to be
updated to "ssize_t"; if the string_list has very many elements,
add_entry() may not find an existing string and add a duplicate at
the end, because the bisection search get_entry_index() uses cannot
move the upper-bound pointer beyond "int".

It would have been better if you culled the parts of the patch you
are not referring to, by the way.

Thanks.