Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.

On Mon Jul 27, 2026 at 6:36 AM IST, Pablo Sabater wrote:

>>  				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.

>>
>>  		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;

> 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.

>>
>>  	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.

>>  	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.

>>
>> 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.