Re: [PATCH 3/4] string-list: change "string_list_find_insert_index" return type to "size_t"
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 10, 2025, 04:57 UTC
- Message-ID
- <aMEFUyRfdeTvPehB@pks.im>
- In-Reply-To
- <xmqq1pofv3dg.fsf@gitster.g>
On Tue, Sep 09, 2025 at 12:21:15PM -0700, Junio C Hamano wrote:
Show 20 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
>
> >> @@ -266,7 +265,7 @@ static struct string_list_item *lookup_prefix(struct string_list *map,
> >> * overlong key would be inserted, which must come after the
> >> * real location of the key if one exists.
> >> */
> >> - while (0 <= --i && i < map->nr) {
> >> + while (i-- > 0 && i < map->nr) {
> >
> > This could simply be `while (i-- && i < map->nr)`.
>
> Yes, especially if the reason why we avoid "not negative" aka "0 <="
> is because the counter is now unsigned, yours is much more intuitive
> way to say "as long as i is not yet zero". Alternatively you could
> say "while (i-- != 0 && ...", but not comparing with 0 is more
> customary.
>
> Better yet, shouldn't we stay away from "i", if the point of the
> change is to make it unsigned, as "i" has a strong connotation with
> "int, the platform natural signed integer type"?For me "i" in a loop typically just means "index" and not "int". So personally, I'm fine with that variable name.
Patrick