Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.
- From
Hardik Kumar <hardikxk@gmail.com>
- Date
- Jul 27, 2026, 06:40 UTC
- Message-ID
- <DK95C0SCPDX3.28ORSCO088KJ9@gmail.com>
- In-Reply-To
- <DK8Y8F4650AW.1XN921ROZW70F@gmail.com>
On Mon Jul 27, 2026 at 6:36 AM IST, Pablo Sabater wrote:
Show 19 quoted lines
>> printf(" (%s%*s %10s",
>> name, pad, "",
>> format_time(ci.author_time,
>> @@ -668,7 +668,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
>>
>> for (e = sb->ent; e; e = e->next) {
>> struct blame_origin *suspect = e->suspect;
>> - int num;
>> + size_t num;
>
> Looking at how num is used, it is reused for multiple things:
> - strlen()
> - utf8_strwidth()
> - line-number sums
>
> The longest_* variables we compare num against are still int.
>
> Can we split num into different variables?
>I think it would be better to just cast the return of `utf8_strwidth` to int instead when assigning it num.
Show 13 quoted lines
>> >> skip_prefix(it->refname, "refs/heads/", &desc); >> skip_prefix(it->refname, "refs/remotes/", &desc); >> diff --git a/builtin/repo.c b/builtin/repo.c >> index 84e012f..47b9191 100644 >> --- a/builtin/repo.c >> +++ b/builtin/repo.c >> @@ -367,7 +367,7 @@ static void stats_table_vaddf(struct stats_table *table, >> struct strbuf buf = STRBUF_INIT; >> struct string_list_item *item; >> char *formatted_name; >> - int name_width; >> + size_t name_width;
Show 16 quoted lines
> Same as above:
>
> if (name_width > table->name_col_width)
>
> I think that these three fields can be promoted safely
>
> struct stats_table {
> [snip]
>
> int name_col_width;
> int value_col_width;
> int unit_col_width;
> };
>
> but check every use of them afterwards for code that still expects an
> int.Changes to the struct field types might generate more signed unsigned warnings leading to changes to fix things which might just not be necessary for this. There probably won't be a use case requiring a very high number for col_width.
Show 43 quoted lines
>>
>> strbuf_vaddf(&buf, format, ap);
>> formatted_name = strbuf_detach(&buf, NULL);
>> @@ -387,12 +387,12 @@ static void stats_table_vaddf(struct stats_table *table,
>> string_list_append_nodup(&table->annotations, strbuf_detach(&buf, NULL));
>> }
>> if (entry->value) {
>> - int value_width = utf8_strwidth(entry->value);
>> + size_t value_width = utf8_strwidth(entry->value);
>
> I feel this one is partially my fault, I wrote these as example output
> of the grep I sent last reroll. But they still need to be checked:
>
>> if (value_width > table->value_col_width)
>
> We are comparing size_t > int.
>
>> table->value_col_width = value_width;
>
> We are narrowing size_t to int.
>
>> }
>> if (entry->unit) {
>> - int unit_width = utf8_strwidth(entry->unit);
>> + size_t unit_width = utf8_strwidth(entry->unit);
>> if (unit_width > table->unit_col_width)
>> table->unit_col_width = unit_width;
>> }
>> @@ -582,8 +582,8 @@ static void stats_table_print_structure(const struct stats_table *table)
>> {
>> const char *name_col_title = _("Repository structure");
>> const char *value_col_title = _("Value");
>> - int title_name_width = utf8_strwidth(name_col_title);
>> - int title_value_width = utf8_strwidth(value_col_title);
>> + size_t title_name_width = utf8_strwidth(name_col_title);
>> + size_t title_value_width = utf8_strwidth(value_col_title);
>
> Same problem, these are compared against int *_col_width locals,
> and:
> value_col_width = title_value_width - unit_col_width
>
> below the context now mixes size_t and int. Promoting the struct fields
> as suggested above fixes all of this at once.Otherwise keeping these as int like before and casting the returns where necessary also works in this case.
Show 21 quoted lines
>> else >> width = options->stat_width ? options->stat_width : 80; >> number_width = decimal_width(max_change) > number_width ? >> @@ -3123,7 +3124,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) >> if (slash) >> name = slash; >> } >> - padding = len - utf8_strwidth(name); >> + padding = len - cast_size_t_to_int(utf8_strwidth(name)); >> if (padding < 0) >> padding = 0; > > The cast doesn't work here because len is also size_t. We could do this > to be sure that there will be no problems: > > size_t name_disp = utf8_strwidth(name); > if (name_disp > len) > padding = 0; > else > padding = cast_size_t_to_int(len - name_disp); >
Changing len back to int resolves this as well as the previous one.
Show 27 quoted lines
>>
>> diff --git a/gettext.c b/gettext.c
>> index 8d08a61..4d5d05e 100644
>> --- a/gettext.c
>> +++ b/gettext.c
>> @@ -129,7 +129,7 @@ void git_setup_gettext(void)
>> }
>>
>> /* return the number of columns of string 's' in current locale */
>> -int gettext_width(const char *s)
>> +size_t gettext_width(const char *s)
>> {
>> static int is_utf8 = -1;
>> if (is_utf8 == -1)
>> diff --git a/gettext.h b/gettext.h
>> index 484cafa..f161a21 100644
>> --- a/gettext.h
>> +++ b/gettext.h
>> @@ -31,7 +31,7 @@
>> #ifndef NO_GETTEXT
>> extern int git_gettext_enabled;
>> void git_setup_gettext(void);
>> -int gettext_width(const char *s);
>> +size_t gettext_width(const char *s);
>
> Careful, this is inside an #ifndef, if we change the signature here,
> the other branch must follow.The implmentations returns either a `utf8_strwidth()` or `strlent()` both of which would return a `size_t`. The function is only called in a single place so I suppose casting it back to int where its called would be better.
Thanks, Hardik.