[PATCH] change utf8_strwidth() return type to size_t

[PATCH] change utf8_strwidth() return type to size_t

[Hardik Kumar]

The patch changes the return types of `utf8_strwidth()` and
`utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions
have been updated in the header file also.

Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
---
 utf8.c | 13 ++++---------
 utf8.h |  4 ++--
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/utf8.c b/utf8.c
index 96460cc..1081573 100644
--- a/utf8.c
+++ b/utf8.c
@@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
  * string, assuming that the string is utf8.  Returns strlen() instead
  * if the string does not look like a valid utf8 string.
  */
-int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
 {
 	const char *orig = string;
 	size_t width = 0;
@@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
 		if (glyph_width > 0)
 			width += glyph_width;
 	}
-
-	/*
-	 * TODO: fix the interface of this function and `utf8_strwidth()` to
-	 * return `size_t` instead of `int`.
-	 */
-	return cast_size_t_to_int(string ? width : len);
+	return (string) ? width : len;
 }
 
-int utf8_strwidth(const char *string)
+size_t utf8_strwidth(const char *string)
 {
 	return utf8_strnwidth(string, strlen(string), 0);
 }
@@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
 		       const char *s)
 {
 	size_t slen = strlen(s);
-	int display_len = utf8_strnwidth(s, slen, 0);
+	size_t display_len = utf8_strnwidth(s, slen, 0);
 	int utf8_compensation = slen - display_len;
 
 	if (display_len >= width) {
diff --git a/utf8.h b/utf8.h
index cf8ecb0..531e968 100644
--- a/utf8.h
+++ b/utf8.h
@@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
 int utf8_width(const char **start, size_t *remainder_p);
-int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
-int utf8_strwidth(const char *string);
+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
+size_t utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);

base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
-- 
2.55.0

Re: [PATCH] change utf8_strwidth() return type to size_t

[René Scharfe]

On 7/26/26 2:34 PM, Hardik Kumar wrote:
> The patch changes the return types of `utf8_strwidth()` and
> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions
> have been updated in the header file also.
> 
> Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
> ---
>  utf8.c | 13 ++++---------
>  utf8.h |  4 ++--
>  2 files changed, 6 insertions(+), 11 deletions(-)

What about callers that still expect int?  Are they all safe without
cast_size_t_to_int()?

> 
> diff --git a/utf8.c b/utf8.c
> index 96460cc..1081573 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
>   * string, assuming that the string is utf8.  Returns strlen() instead
>   * if the string does not look like a valid utf8 string.
>   */
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>  {
>  	const char *orig = string;
>  	size_t width = 0;
> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>  		if (glyph_width > 0)
>  			width += glyph_width;
>  	}
> -
> -	/*
> -	 * TODO: fix the interface of this function and `utf8_strwidth()` to
> -	 * return `size_t` instead of `int`.
> -	 */
> -	return cast_size_t_to_int(string ? width : len);
> +	return (string) ? width : len;

Nit: Why the parentheses around "string"?

>  }
>  
> -int utf8_strwidth(const char *string)
> +size_t utf8_strwidth(const char *string)
>  {
>  	return utf8_strnwidth(string, strlen(string), 0);
>  }
> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
>  		       const char *s)
>  {
>  	size_t slen = strlen(s);
> -	int display_len = utf8_strnwidth(s, slen, 0);
> +	size_t display_len = utf8_strnwidth(s, slen, 0);
>  	int utf8_compensation = slen - display_len;
>  
>  	if (display_len >= width) {
> diff --git a/utf8.h b/utf8.h
> index cf8ecb0..531e968 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
>  
>  size_t display_mode_esc_sequence_len(const char *s);
>  int utf8_width(const char **start, size_t *remainder_p);
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> -int utf8_strwidth(const char *string);
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> +size_t utf8_strwidth(const char *string);
>  int is_utf8(const char *text);
>  int is_encoding_utf8(const char *name);
>  int same_encoding(const char *, const char *);
> 
> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca

Re: [PATCH] change utf8_strwidth() return type to size_t

[Pablo Sabater]

On Sun Jul 26, 2026 at 2:34 PM CEST, Hardik Kumar wrote:
> The patch changes the return types of `utf8_strwidth()` and

Regarding the presentation: "The patch changes...", try to avoid this
pattern, I think something like this would fit better:

utf8_strwidth() and utf8_strnwidth() return int, even though the value
they return is always non-negative:

- utf8_strnwidth() accumulates the width into a size_t and otherwise
  returns its size_t len parameter,
- utf8_strwidth() just forwards its result.

Change their signatures to return size_t instead.

If you want to mention the TODO, I would add it after the '---'.

> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions
> have been updated in the header file also.
>
> Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
> ---
>  utf8.c | 13 ++++---------
>  utf8.h |  4 ++--
>  2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/utf8.c b/utf8.c
> index 96460cc..1081573 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
>   * string, assuming that the string is utf8.  Returns strlen() instead
>   * if the string does not look like a valid utf8 string.
>   */
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>  {
>  	const char *orig = string;
>  	size_t width = 0;
> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>  		if (glyph_width > 0)
>  			width += glyph_width;
>  	}
> -
> -	/*
> -	 * TODO: fix the interface of this function and `utf8_strwidth()` to
> -	 * return `size_t` instead of `int`.
> -	 */
> -	return cast_size_t_to_int(string ? width : len);
> +	return (string) ? width : len;

nit: parentheses at "(string)" are unnecessary.

Also, cast_size_t_to_int() had an overflow check, we need to be sure
that no caller relies on that check. If you have checked for that,
please mention it in the commit message.

>  }
>
> -int utf8_strwidth(const char *string)
> +size_t utf8_strwidth(const char *string)
>  {
>  	return utf8_strnwidth(string, strlen(string), 0);
>  }
> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
>  		       const char *s)
>  {
>  	size_t slen = strlen(s);
> -	int display_len = utf8_strnwidth(s, slen, 0);
> +	size_t display_len = utf8_strnwidth(s, slen, 0);

We are fixing a caller here and that is correct.
But these functions that we've changed in this patch are called
throughout the codebase, we should fix those callers too.

We can check who their callers are with:

  git grep -n -E 'utf8_str.?width'

builtin/repo.c:390:             int value_width = utf8_strwidth(entry->value);
builtin/repo.c:395:             int unit_width = utf8_strwidth(entry->unit);
builtin/repo.c:585:     int title_name_width = utf8_strwidth(name_col_title);
builtin/repo.c:586:     int title_value_width = utf8_strwidth(value_col_title);

(there are more)

From what I reviewed, no caller will break because of this, but I think
we should fix it for consistency.

>  	int utf8_compensation = slen - display_len;
>
>  	if (display_len >= width) {
> diff --git a/utf8.h b/utf8.h
> index cf8ecb0..531e968 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
>
>  size_t display_mode_esc_sequence_len(const char *s);
>  int utf8_width(const char **start, size_t *remainder_p);
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> -int utf8_strwidth(const char *string);
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> +size_t utf8_strwidth(const char *string);
>  int is_utf8(const char *text);
>  int is_encoding_utf8(const char *name);
>  int same_encoding(const char *, const char *);
>
> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca

The signature change looks ok.

Regards,
Pablo

Re: [PATCH] change utf8_strwidth() return type to size_t

[Hardik Kumar]

On Sun Jul 26, 2026 at 7:11 PM IST, René Scharfe wrote:
> On 7/26/26 2:34 PM, Hardik Kumar wrote:
>> The patch changes the return types of `utf8_strwidth()` and
>> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions
>> have been updated in the header file also.
>> 
>> Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
>> ---
>>  utf8.c | 13 ++++---------
>>  utf8.h |  4 ++--
>>  2 files changed, 6 insertions(+), 11 deletions(-)
>
> What about callers that still expect int?  Are they all safe without
> cast_size_t_to_int()?
>
The return type should be implicitly converted back to int for all the
locations its being called at. If implicit conversions are not
encouraged I could change the types of the variables at the call sites?

>> 
>> diff --git a/utf8.c b/utf8.c
>> index 96460cc..1081573 100644
>> --- a/utf8.c
>> +++ b/utf8.c
>> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
>>   * string, assuming that the string is utf8.  Returns strlen() instead
>>   * if the string does not look like a valid utf8 string.
>>   */
>> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>>  {
>>  	const char *orig = string;
>>  	size_t width = 0;
>> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>>  		if (glyph_width > 0)
>>  			width += glyph_width;
>>  	}
>> -
>> -	/*
>> -	 * TODO: fix the interface of this function and `utf8_strwidth()` to
>> -	 * return `size_t` instead of `int`.
>> -	 */
>> -	return cast_size_t_to_int(string ? width : len);
>> +	return (string) ? width : len;
>
> Nit: Why the parentheses around "string"?
>
Bad habit I'll drop them in v2. Makes it obvious we are expecting a bool
value here.

>>  }
>>  
>> -int utf8_strwidth(const char *string)
>> +size_t utf8_strwidth(const char *string)
>>  {
>>  	return utf8_strnwidth(string, strlen(string), 0);
>>  }
>> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
>>  		       const char *s)
>>  {
>>  	size_t slen = strlen(s);
>> -	int display_len = utf8_strnwidth(s, slen, 0);
>> +	size_t display_len = utf8_strnwidth(s, slen, 0);
>>  	int utf8_compensation = slen - display_len;
>>  
>>  	if (display_len >= width) {
>> diff --git a/utf8.h b/utf8.h
>> index cf8ecb0..531e968 100644
>> --- a/utf8.h
>> +++ b/utf8.h
>> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
>>  
>>  size_t display_mode_esc_sequence_len(const char *s);
>>  int utf8_width(const char **start, size_t *remainder_p);
>> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
>> -int utf8_strwidth(const char *string);
>> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
>> +size_t utf8_strwidth(const char *string);
>>  int is_utf8(const char *text);
>>  int is_encoding_utf8(const char *name);
>>  int same_encoding(const char *, const char *);
>> 
>> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca

Re: [PATCH] change utf8_strwidth() return type to size_t

[Hardik Kumar]

Noted! I'll write up as suggested in v2 for this patch.

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

[Hardik Kumar]

utf8_strwidth() and utf8_strnwidth() return int, even though the
return value is always non-negative:

- utf8_strnwidth() accumulates the width into a size_t and otherwise
  returns its size_t len parameter,
- utf8_strwidth() just forwards its result.

Change their signatures to return size_t instead.

Update the types of the variables the said method is used to avoid
potential UB caused by implicit conversion from size_t to int.

The returned values from `utf8_strwidth()` are casted to int at places
where it was falling tests or required other changes.

Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
---
Changes in v2:
- reworked types for utf8_strwidth and its sites of usage.
- removed redundant parens around `string`.
- updated commit message for better explaining the patch.

 builtin/blame.c  |  4 ++--
 builtin/branch.c |  2 +-
 builtin/repo.c   | 10 +++++-----
 column.c         |  2 +-
 diff.c           |  7 ++++---
 gettext.c        |  2 +-
 gettext.h        |  2 +-
 pretty.c         |  5 +++--
 utf8.c           | 13 ++++---------
 utf8.h           |  4 ++--
 wt-status.c      |  8 ++++----
 11 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 48d5251..2d24b63 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
 					name = ci.author_mail.buf;
 				else
 					name = ci.author.buf;
-				pad = longest_author - utf8_strwidth(name);
+				pad = longest_author - cast_size_t_to_int(utf8_strwidth(name));
 				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;
 		size_t marks_count = count_marks(e, *option);
 
 		if (max_marks_count < marks_count)
diff --git a/builtin/branch.c b/builtin/branch.c
index dede60d..514ba64 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -354,7 +354,7 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
 	for (i = 0; i < refs->nr; i++) {
 		struct ref_array_item *it = refs->items[i];
 		const char *desc = it->refname;
-		int w;
+		size_t w;
 
 		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;
 
 	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);
 		if (value_width > table->value_col_width)
 			table->value_col_width = value_width;
 	}
 	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);
 	int name_col_width = table->name_col_width;
 	int value_col_width = table->value_col_width;
 	int unit_col_width = table->unit_col_width;
diff --git a/column.c b/column.c
index 93fae31..6b7f921 100644
--- a/column.c
+++ b/column.c
@@ -24,7 +24,7 @@ struct column_data {
 };
 
 /* return length of 's' in letters, ANSI escapes stripped */
-static int item_length(const char *s)
+static size_t item_length(const char *s)
 {
 	return utf8_strnwidth(s, strlen(s), 1);
 }
diff --git a/diff.c b/diff.c
index 589c196..4887958 100644
--- a/diff.c
+++ b/diff.c
@@ -2952,7 +2952,8 @@ static int utf8_ish_width(const char **start)
 
 static void show_stats(struct diffstat_t *data, struct diff_options *options)
 {
-	int i, len, add, del, adds = 0, dels = 0;
+	int i, add, del, adds = 0, dels = 0;
+	size_t len;
 	uintmax_t max_change = 0, max_len = 0;
 	int total_files = data->nr, count;
 	int width, name_width, graph_width, number_width = 0, bin_width = 0;
@@ -3037,7 +3038,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * making the line longer than the maximum width.
 	 */
 	if (options->stat_width == -1)
-		width = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);
+		width = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));
 	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;
 
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);
 #else
 #define git_gettext_enabled (0)
 static inline void git_setup_gettext(void)
diff --git a/pretty.c b/pretty.c
index d8a9f37..f7d392d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1805,11 +1805,12 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 {
 	struct strbuf local_sb = STRBUF_INIT;
 	size_t total_consumed = 0;
-	int len, padding = c->padding;
+	int padding = c->padding;
+	size_t len;
 
 	if (padding < 0) {
 		const char *start = strrchr(sb->buf, '\n');
-		int occupied;
+		size_t occupied;
 		if (!start)
 			start = sb->buf;
 		occupied = utf8_strnwidth(start, strlen(start), 1);
diff --git a/utf8.c b/utf8.c
index 96460cc..cefaefe 100644
--- a/utf8.c
+++ b/utf8.c
@@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
  * string, assuming that the string is utf8.  Returns strlen() instead
  * if the string does not look like a valid utf8 string.
  */
-int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
 {
 	const char *orig = string;
 	size_t width = 0;
@@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
 		if (glyph_width > 0)
 			width += glyph_width;
 	}
-
-	/*
-	 * TODO: fix the interface of this function and `utf8_strwidth()` to
-	 * return `size_t` instead of `int`.
-	 */
-	return cast_size_t_to_int(string ? width : len);
+	return string ? width : len;
 }
 
-int utf8_strwidth(const char *string)
+size_t utf8_strwidth(const char *string)
 {
 	return utf8_strnwidth(string, strlen(string), 0);
 }
@@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
 		       const char *s)
 {
 	size_t slen = strlen(s);
-	int display_len = utf8_strnwidth(s, slen, 0);
+	size_t display_len = utf8_strnwidth(s, slen, 0);
 	int utf8_compensation = slen - display_len;
 
 	if (display_len >= width) {
diff --git a/utf8.h b/utf8.h
index cf8ecb0..531e968 100644
--- a/utf8.h
+++ b/utf8.h
@@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
 int utf8_width(const char **start, size_t *remainder_p);
-int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
-int utf8_strwidth(const char *string);
+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
+size_t utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);
diff --git a/wt-status.c b/wt-status.c
index 58461e0..0e1e32d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -331,9 +331,9 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)
 
 	for (i = minval; i <= maxval; i++) {
 		const char *s = label(i);
-		int len = s ? utf8_strwidth(s) : 0;
+		size_t len = s ? utf8_strwidth(s) : 0;
 		if (len > result)
-			result = len;
+			result = cast_size_t_to_int(len);
 	}
 	return result;
 }
@@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,
 	status_printf(s, color(WT_STATUS_HEADER, s), "\t");
 
 	how = wt_status_unmerged_status_string(d->stagemask);
-	len = label_width - utf8_strwidth(how);
+	len = label_width - cast_size_t_to_int(utf8_strwidth(how));
 	status_printf_more(s, c, "%s%.*s%s\n", how, len, padding, one);
 	strbuf_release(&onebuf);
 }
@@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
 	what = wt_status_diff_status_string(status);
 	if (!what)
 		BUG("unhandled diff status %c", status);
-	len = label_width - utf8_strwidth(what);
+	len = label_width - cast_size_t_to_int(utf8_strwidth(what));
 	assert(len >= 0);
 	if (one_name != two_name)
 		status_printf_more(s, c, "%s%.*s%s -> %s",
-- 
2.55.0

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

[Junio C Hamano]

Hardik Kumar <hardikxk@gmail.com> writes:

> utf8_strwidth() and utf8_strnwidth() return int, even though the
> return value is always non-negative:
>
> - utf8_strnwidth() accumulates the width into a size_t and otherwise
>   returns its size_t len parameter,
> - utf8_strwidth() just forwards its result.
>
> Change their signatures to return size_t instead.
>
> Update the types of the variables the said method is used to avoid
> potential UB caused by implicit conversion from size_t to int.

The goal looks attractive on the surface, and the change to make
utf8_strwidth() and utf8_strnwidth() return 'size_t' clears an
existing TODO.  However, the updates to the call sites to support
this change introduce several bugs due to unsigned integer underflow
and incorrect mixed-sign comparisons.

Consider just one example:

> diff --git a/diff.c b/diff.c
> index 589c196..4887958 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2952,7 +2952,8 @@ static int utf8_ish_width(const char **start)
>  
>  static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  {
> -	int i, len, add, del, adds = 0, dels = 0;
> +	int i, add, del, adds = 0, dels = 0;
> +	size_t len;
>  	uintmax_t max_change = 0, max_len = 0;
>  	int total_files = data->nr, count;
>  	int width, name_width, graph_width, number_width = 0, bin_width = 0;

The above change impacts code later in the function (among other
things):

		/*
		 * "scale" the filename
		 */
		len = name_width;
		name_len = utf8_strwidth(name);
		if (name_width < name_len) {
			char *slash;
			prefix = "...";
			len -= 3;
			if (len < 0)
				len = 0;

Here, 'len' used to be an 'int', but now it is 'size_t', which is
unsigned.  The safeguard to prevent 'len' from going down to an
unacceptably low value by clipping it to 0 never triggers, because
'if (len < 0)' can never be true.  If len is less than 3, len -= 3
will result in a fairly large value, and the subsequent computation
would go bananas to see a value with little relation to name_len.

Another example.

> diff --git a/pretty.c b/pretty.c
> index d8a9f37..f7d392d 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1805,11 +1805,12 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>  {
>  	struct strbuf local_sb = STRBUF_INIT;
>  	size_t total_consumed = 0;
> -	int len, padding = c->padding;
> +	int padding = c->padding;
> +	size_t len;
>  
>  	if (padding < 0) {
>  		const char *start = strrchr(sb->buf, '\n');
> -		int occupied;
> +		size_t occupied;
>  		if (!start)
>  			start = sb->buf;
>  		occupied = utf8_strnwidth(start, strlen(start), 1);

After this post-context, 'occupied' is incremented, and then we have
this:

		padding = (-padding) - occupied;

If 'occupied' is sufficiently large, 'padding' can become negative
here.  Because padding remains an 'int' and can become negative, it
impacts code a bit further down in the same function (among other
similar comparisons):

	if (c->flush_type == flush_left_and_steal) {
		const char *ch = sb->buf + sb->len - 1;
		while (len > padding && ch > sb->buf) {
			const char *p;
			if (*ch == ' ') {
				ch--;
				padding++;
				continue;
			}

We compare 'len' and 'padding', first promoting 'padding' to
'size_t', so when 'padding' is negative, we compare 'len' with a
fairly large number due to unsigned wraparound.  We will fail to
"steal" spaces as we will not loop here.

I will stop here.  What makes reviewing this change so unpleasant is
that on the surface, changing variable definitions to flip int to
size_t looks pretty, yet the real breakage appears in places that
are not shown in the patch at all.

So, this needs more work to become acceptable, I am afraid.

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

[Pablo Sabater]

[+cc Junio, who reviewed this while I was writing mine, to keep him in
the thread]

Hi!

Note that you have sent v2 in-reply-to my review from last version, not
to the v1.

This Patch does not compile with DEVELOPER=1:

  $ make DEVELOPER=1

  builtin/blame.c:681:20: error: comparison of integers of different signs: 'int' and 'size_t'
        (aka 'unsigned long') [-Werror,-Wsign-compare]
    681 |                 if (longest_file < num)
        |                     ~~~~~~~~~~~~ ^ ~~~
  builtin/blame.c:691:23: error: comparison of integers of different signs: 'int' and 'size_t'
        (aka 'unsigned long') [-Werror,-Wsign-compare]
    691 |                         if (longest_author < num)
        |                             ~~~~~~~~~~~~~~ ^ ~~~
  builtin/blame.c:696:25: error: comparison of integers of different signs: 'int' and 'size_t'
        (aka 'unsigned long') [-Werror,-Wsign-compare]
    696 |                 if (longest_src_lines < num)
        |                     ~~~~~~~~~~~~~~~~~ ^ ~~~
  builtin/blame.c:699:25: error: comparison of integers of different signs: 'int' and 'size_t'
        (aka 'unsigned long') [-Werror,-Wsign-compare]
    699 |                 if (longest_dst_lines < num)
        |                     ~~~~~~~~~~~~~~~~~ ^ ~~~

Also note that many files have DISABLE_SIGN_COMPARE_WARNINGS which hides
from us this errors.

That's why we have to be extra careful when changing signatures of
functions accross the codebase.

The title is being too explicit, when a function signature changes, it
is expected that their callers will change, no need to add it to the
title. What about:

  utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

does it work?

On Sun Jul 26, 2026 at 9:57 PM CEST, Hardik Kumar wrote:
> utf8_strwidth() and utf8_strnwidth() return int, even though the
> return value is always non-negative:
>
> - utf8_strnwidth() accumulates the width into a size_t and otherwise
>   returns its size_t len parameter,

nit: change the comma for a dot.

> - utf8_strwidth() just forwards its result.
>
> Change their signatures to return size_t instead.
>
> Update the types of the variables the said method is used to avoid
> potential UB caused by implicit conversion from size_t to int.

This is not correct and it reads a bit off, what about:

  Update the types of the variables where these functions are used, to
  avoid the implicit conversion from size_t to int.

This is not correct because the implicit conversion from size_t to
int is not undefined behavior.

>
> The returned values from `utf8_strwidth()` are casted to int at places
> where it was falling tests or required other changes.

nit: s/casted/cast/
nit: s/falling/failing/

>
> Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
> ---
> Changes in v2:
> - reworked types for utf8_strwidth and its sites of usage.
> - removed redundant parens around `string`.
> - updated commit message for better explaining the patch.
>
>  builtin/blame.c  |  4 ++--
>  builtin/branch.c |  2 +-
>  builtin/repo.c   | 10 +++++-----
>  column.c         |  2 +-
>  diff.c           |  7 ++++---
>  gettext.c        |  2 +-
>  gettext.h        |  2 +-
>  pretty.c         |  5 +++--
>  utf8.c           | 13 ++++---------
>  utf8.h           |  4 ++--
>  wt-status.c      |  8 ++++----
>  11 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 48d5251..2d24b63 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
>  					name = ci.author_mail.buf;
>  				else
>  					name = ci.author.buf;
> -				pad = longest_author - utf8_strwidth(name);
> +				pad = longest_author - cast_size_t_to_int(utf8_strwidth(name));

This one is fine.

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

>  		size_t marks_count = count_marks(e, *option);
>
>  		if (max_marks_count < marks_count)
> diff --git a/builtin/branch.c b/builtin/branch.c
> index dede60d..514ba64 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -354,7 +354,7 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
>  	for (i = 0; i < refs->nr; i++) {
>  		struct ref_array_item *it = refs->items[i];
>  		const char *desc = it->refname;
> -		int w;
> +		size_t w;

w receives utf8_strwidth() but later we have:

  if (w > max)

This is now size_t > int.
Here I would keep w int and cast.

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

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


>  	int name_col_width = table->name_col_width;
>  	int value_col_width = table->value_col_width;
>  	int unit_col_width = table->unit_col_width;
> diff --git a/column.c b/column.c
> index 93fae31..6b7f921 100644
> --- a/column.c
> +++ b/column.c
> @@ -24,7 +24,7 @@ struct column_data {
>  };
>
>  /* return length of 's' in letters, ANSI escapes stripped */
> -static int item_length(const char *s)
> +static size_t item_length(const char *s)
>  {
>  	return utf8_strnwidth(s, strlen(s), 1);
>  }

item_length() has only one caller, which stores the result into an
int *, so the value gets narrowed right back to int and this change
buys nothing. Keep returning int and cast inside.

> diff --git a/diff.c b/diff.c
> index 589c196..4887958 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2952,7 +2952,8 @@ static int utf8_ish_width(const char **start)
>
>  static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  {
> -	int i, len, add, del, adds = 0, dels = 0;
> +	int i, add, del, adds = 0, dels = 0;
> +	size_t len;

This will have problems below:

	[snip]

	len = name_width;
	name_len = utf8_strwidth(name);
	if (name_width < name_len) {
		prefix = "...";
		len -= 3;
		if (len < 0)
			len = 0;
		while (name_len > len && *name)

	[snip]

if (len < 0) will always be false for a size_t, becoming dead code.
Also name_len > len is int > size_t.

Let's step back a bit.
name_len receives utf8_strwidth(), let's make it size_t too, and
compare against len so the types stay consistent. The dead code can
become:

	len = len > 3 ? len - 3 : 0;

We would also have to change the check:

	if (name_width < name_len)
to:
	if (len < name_len)

>  	uintmax_t max_change = 0, max_len = 0;
>  	int total_files = data->nr, count;
>  	int width, name_width, graph_width, number_width = 0, bin_width = 0;
> @@ -3037,7 +3038,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>  	 * making the line longer than the maximum width.
>  	 */
>  	if (options->stat_width == -1)
> -		width = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);
> +		width = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));

This one is fine.

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

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

>  #else
>  #define git_gettext_enabled (0)
>  static inline void git_setup_gettext(void)
> diff --git a/pretty.c b/pretty.c
> index d8a9f37..f7d392d 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1805,11 +1805,12 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>  {
>  	struct strbuf local_sb = STRBUF_INIT;
>  	size_t total_consumed = 0;
> -	int len, padding = c->padding;
> +	int padding = c->padding;
> +	size_t len;
>
>  	if (padding < 0) {
>  		const char *start = strrchr(sb->buf, '\n');
> -		int occupied;
> +		size_t occupied;
>  		if (!start)
>  			start = sb->buf;
>  		occupied = utf8_strnwidth(start, strlen(start), 1);

padding is signed on purpose, so we can't have len as size_t.
Later we have len > padding.
Keep len and occupied as int, casting the utf8_strnwidth() results
instead.

> diff --git a/utf8.c b/utf8.c
> index 96460cc..cefaefe 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
>   * string, assuming that the string is utf8.  Returns strlen() instead
>   * if the string does not look like a valid utf8 string.
>   */
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>  {
>  	const char *orig = string;
>  	size_t width = 0;
> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>  		if (glyph_width > 0)
>  			width += glyph_width;
>  	}
> -
> -	/*
> -	 * TODO: fix the interface of this function and `utf8_strwidth()` to
> -	 * return `size_t` instead of `int`.
> -	 */
> -	return cast_size_t_to_int(string ? width : len);
> +	return string ? width : len;
>  }

This is good.

>
> -int utf8_strwidth(const char *string)
> +size_t utf8_strwidth(const char *string)
>  {
>  	return utf8_strnwidth(string, strlen(string), 0);
>  }
> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
>  		       const char *s)
>  {
>  	size_t slen = strlen(s);
> -	int display_len = utf8_strnwidth(s, slen, 0);
> +	size_t display_len = utf8_strnwidth(s, slen, 0);
>  	int utf8_compensation = slen - display_len;

This is fine.

>
>  	if (display_len >= width) {
> diff --git a/utf8.h b/utf8.h
> index cf8ecb0..531e968 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
>
>  size_t display_mode_esc_sequence_len(const char *s);
>  int utf8_width(const char **start, size_t *remainder_p);
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> -int utf8_strwidth(const char *string);
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> +size_t utf8_strwidth(const char *string);
>  int is_utf8(const char *text);
>  int is_encoding_utf8(const char *name);
>  int same_encoding(const char *, const char *);
> diff --git a/wt-status.c b/wt-status.c
> index 58461e0..0e1e32d 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -331,9 +331,9 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)
>
>  	for (i = minval; i <= maxval; i++) {
>  		const char *s = label(i);
> -		int len = s ? utf8_strwidth(s) : 0;
> +		size_t len = s ? utf8_strwidth(s) : 0;
>  		if (len > result)
> -			result = len;
> +			result = cast_size_t_to_int(len);
>  	}
>  	return result;
>  }

This function returns non-negative, I would say that the TODO applies to
it as well because it returns 0 or utf8_strwidth(). Cleaning it to return
size_t would help in some steps below:

> @@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,
>  	status_printf(s, color(WT_STATUS_HEADER, s), "\t");
>
>  	how = wt_status_unmerged_status_string(d->stagemask);
> -	len = label_width - utf8_strwidth(how);
> +	len = label_width - cast_size_t_to_int(utf8_strwidth(how));
>  	status_printf_more(s, c, "%s%.*s%s\n", how, len, padding, one);
>  	strbuf_release(&onebuf);
>  }
> @@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
>  	what = wt_status_diff_status_string(status);
>  	if (!what)
>  		BUG("unhandled diff status %c", status);
> -	len = label_width - utf8_strwidth(what);
> +	len = label_width - cast_size_t_to_int(utf8_strwidth(what));
>  	assert(len >= 0);

We can see here that len has to be non-negative. label_width comes from
maxwidth(). With the cleanup I suggested, maxwidth() returns size_t and
so does label_width.

We can have the same safety with:

	size_t what_width = utf8_strwidth(what);

	if (what_width > label_width)
		BUG("label wider than column");
	len = cast_size_t_to_int(label_width - what_width);

You need the cast at the end anyway because len is used as the
precision in "%.*s".

But I think this way leaves the code cleaner.

>  	if (one_name != two_name)
>  		status_printf_more(s, c, "%s%.*s%s -> %s",

Please know that the hunks of code I suggest might not be the direct
solution, so don't take them directly. I do this because I don't want
to write whole functions and I think that if I just write what's
relevant it will be easier to understand.

Also, part of your work as author is to verify what you add and be able
to defend it, which means taking what others say (including this
review) with a grain of salt, everyone can make mistakes. You can run
the tests on your own to check before sending the next version.

Nice work,
Pablo

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> The goal looks attractive on the surface, and the change to make
> utf8_strwidth() and utf8_strnwidth() return 'size_t' clears an
> existing TODO.  However, the updates to the call sites to support
> this change introduce several bugs due to unsigned integer underflow
> and incorrect mixed-sign comparisons.

Having said that, we need to remember that these two functions are
not designed for anything more than what fits on a single line.  The
only reason they exist in our codebase is because their callers want
to measure the display width of a string, so that they can align
elements on a line vertically with the corresponding elements on the
previous and next lines.

This does not mean we do not need to support more than 80 columns
;-), but they surely do not have to support a 2-billion-column-wide
display.

Quite honestly, I have to say that this topic has a very low
expected benefit in practice, while it costs us quite a lot by
having to carefully code and even more carefully review.  If we have
to endure so many new bugs in the callers just to clear an existing
TODO, we might be better off not doing so and relying on the "safe
cast from size_t down to int that barfs if the quantity does not fit
in an int" protection.

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

[Hardik Kumar]

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.

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

[Hardik Kumar]

On Mon Jul 27, 2026 at 9:32 AM IST, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> Quite honestly, I have to say that this topic has a very low
> expected benefit in practice, while it costs us quite a lot by
> having to carefully code and even more carefully review.  If we have
> to endure so many new bugs in the callers just to clear an existing
> TODO, we might be better off not doing so and relying on the "safe
> cast from size_t down to int that barfs if the quantity does not fit
> in an int" protection.

I agree that while this might not net something significant and yes
going through this is difficult but, many places it would much rather
make sense having an unsigned int as mostly its rare that we would be
dealing with negatives except in a few cases which you highlighted
before and others I got when reworking.

I would like to send up a patch with some better changes done. I had
previously not built and tested with warnings enabled apologies for
that I assumed the defaults to enable them without explicit args.

Thanks,
Hardik

[PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Hardik Kumar]

utf8_strwidth() and utf8_strnwidth() return int, even though the
return value is always non-negative:

- utf8_strnwidth() accumulates the width into a size_t and otherwise
  returns its size_t len parameter.
- utf8_strwidth() just forwards its result.

Change their signatures to return size_t instead.

Update the types of the variables where these method is used to avoid
implicit conversion from size_t to int.

The return values from `utf8_strwidth()` are cast to int where
negative values are expected or depend on other int variables.

Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
---
 builtin/blame.c |  6 +++---
 builtin/fetch.c |  2 +-
 builtin/repo.c  | 10 +++++-----
 column.c        |  2 +-
 diff.c          |  8 ++++----
 gettext.c       |  2 +-
 gettext.h       |  2 +-
 pretty.c        |  4 ++--
 utf8.c          | 13 ++++---------
 utf8.h          |  4 ++--
 wt-status.c     | 10 +++++-----
 11 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 48d5251..83e4dd6 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
 					name = ci.author_mail.buf;
 				else
 					name = ci.author.buf;
-				pad = longest_author - utf8_strwidth(name);
+				pad = longest_author - cast_size_t_to_int(utf8_strwidth(name));
 				printf(" (%s%*s %10s",
 				       name, pad, "",
 				       format_time(ci.author_time,
@@ -685,9 +685,9 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci);
 			if (*option & OUTPUT_SHOW_EMAIL)
-				num = utf8_strwidth(ci.author_mail.buf);
+				num = cast_size_t_to_int(utf8_strwidth(ci.author_mail.buf));
 			else
-				num = utf8_strwidth(ci.author.buf);
+				num = cast_size_t_to_int(utf8_strwidth(ci.author.buf));
 			if (longest_author < num)
 				longest_author = num;
 			commit_info_destroy(&ci);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 775a797..c4ae95f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -850,7 +850,7 @@ static void display_ref_update(struct display_state *display_state, char code,
 			display_state->shown_url = 1;
 		}
 
-		width = (summary_width + strlen(summary) - gettext_width(summary));
+		width = (summary_width + strlen(summary) - cast_size_t_to_int(gettext_width(summary)));
 		remote = prettify_refname(remote);
 		local = prettify_refname(local);
 
diff --git a/builtin/repo.c b/builtin/repo.c
index 84e012f..9c7ad8c 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -371,7 +371,7 @@ static void stats_table_vaddf(struct stats_table *table,
 
 	strbuf_vaddf(&buf, format, ap);
 	formatted_name = strbuf_detach(&buf, NULL);
-	name_width = utf8_strwidth(formatted_name);
+	name_width = cast_size_t_to_int(utf8_strwidth(formatted_name));
 
 	item = string_list_append_nodup(&table->rows, formatted_name);
 	item->util = entry;
@@ -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);
+		int value_width = cast_size_t_to_int(utf8_strwidth(entry->value));
 		if (value_width > table->value_col_width)
 			table->value_col_width = value_width;
 	}
 	if (entry->unit) {
-		int unit_width = utf8_strwidth(entry->unit);
+		int unit_width = cast_size_t_to_int(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);
+	int title_name_width = cast_size_t_to_int(utf8_strwidth(name_col_title));
+	int title_value_width = cast_size_t_to_int(utf8_strwidth(value_col_title));
 	int name_col_width = table->name_col_width;
 	int value_col_width = table->value_col_width;
 	int unit_col_width = table->unit_col_width;
diff --git a/column.c b/column.c
index 93fae31..a63d040 100644
--- a/column.c
+++ b/column.c
@@ -26,7 +26,7 @@ struct column_data {
 /* return length of 's' in letters, ANSI escapes stripped */
 static int item_length(const char *s)
 {
-	return utf8_strnwidth(s, strlen(s), 1);
+	return cast_size_t_to_int(utf8_strnwidth(s, strlen(s), 1));
 }
 
 /*
diff --git a/diff.c b/diff.c
index 589c196..205fedf 100644
--- a/diff.c
+++ b/diff.c
@@ -2982,7 +2982,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 			continue;
 		}
 		fill_print_name(file);
-		len = utf8_strwidth(file->print_name);
+		len = cast_size_t_to_int(utf8_strwidth(file->print_name));
 		if (max_len < len)
 			max_len = len;
 
@@ -3037,7 +3037,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * making the line longer than the maximum width.
 	 */
 	if (options->stat_width == -1)
-		width = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);
+		width = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));
 	else
 		width = options->stat_width ? options->stat_width : 80;
 	number_width = decimal_width(max_change) > number_width ?
@@ -3108,7 +3108,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 		 * "scale" the filename
 		 */
 		len = name_width;
-		name_len = utf8_strwidth(name);
+		name_len = cast_size_t_to_int(utf8_strwidth(name));
 		if (name_width < name_len) {
 			char *slash;
 			prefix = "...";
@@ -3123,7 +3123,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;
 
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);
 #else
 #define git_gettext_enabled (0)
 static inline void git_setup_gettext(void)
diff --git a/pretty.c b/pretty.c
index d8a9f37..83d4e86 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1809,7 +1809,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 
 	if (padding < 0) {
 		const char *start = strrchr(sb->buf, '\n');
-		int occupied;
+		size_t occupied;
 		if (!start)
 			start = sb->buf;
 		occupied = utf8_strnwidth(start, strlen(start), 1);
@@ -1830,7 +1830,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 		placeholder++;
 		total_consumed++;
 	}
-	len = utf8_strnwidth(local_sb.buf, local_sb.len, 1);
+	len = cast_size_t_to_int(utf8_strnwidth(local_sb.buf, local_sb.len, 1));
 
 	if (c->flush_type == flush_left_and_steal) {
 		const char *ch = sb->buf + sb->len - 1;
diff --git a/utf8.c b/utf8.c
index 96460cc..cefaefe 100644
--- a/utf8.c
+++ b/utf8.c
@@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
  * string, assuming that the string is utf8.  Returns strlen() instead
  * if the string does not look like a valid utf8 string.
  */
-int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
 {
 	const char *orig = string;
 	size_t width = 0;
@@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
 		if (glyph_width > 0)
 			width += glyph_width;
 	}
-
-	/*
-	 * TODO: fix the interface of this function and `utf8_strwidth()` to
-	 * return `size_t` instead of `int`.
-	 */
-	return cast_size_t_to_int(string ? width : len);
+	return string ? width : len;
 }
 
-int utf8_strwidth(const char *string)
+size_t utf8_strwidth(const char *string)
 {
 	return utf8_strnwidth(string, strlen(string), 0);
 }
@@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
 		       const char *s)
 {
 	size_t slen = strlen(s);
-	int display_len = utf8_strnwidth(s, slen, 0);
+	size_t display_len = utf8_strnwidth(s, slen, 0);
 	int utf8_compensation = slen - display_len;
 
 	if (display_len >= width) {
diff --git a/utf8.h b/utf8.h
index cf8ecb0..531e968 100644
--- a/utf8.h
+++ b/utf8.h
@@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 size_t display_mode_esc_sequence_len(const char *s);
 int utf8_width(const char **start, size_t *remainder_p);
-int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
-int utf8_strwidth(const char *string);
+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
+size_t utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 int same_encoding(const char *, const char *);
diff --git a/wt-status.c b/wt-status.c
index 58461e0..672f83b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -325,13 +325,13 @@ static const char *wt_status_diff_status_string(int status)
 	}
 }
 
-static int maxwidth(const char *(*label)(int), int minval, int maxval)
+static size_t maxwidth(const char *(*label)(int), int minval, int maxval)
 {
 	int result = 0, i;
 
 	for (i = minval; i <= maxval; i++) {
 		const char *s = label(i);
-		int len = s ? utf8_strwidth(s) : 0;
+		size_t len = s ? utf8_strwidth(s) : 0;
 		if (len > result)
 			result = len;
 	}
@@ -345,7 +345,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,
 	struct wt_status_change_data *d = it->util;
 	struct strbuf onebuf = STRBUF_INIT;
 	static char *padding;
-	static int label_width;
+	static size_t label_width;
 	const char *one, *how;
 	int len;
 
@@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,
 	status_printf(s, color(WT_STATUS_HEADER, s), "\t");
 
 	how = wt_status_unmerged_status_string(d->stagemask);
-	len = label_width - utf8_strwidth(how);
+	len = label_width - cast_size_t_to_int(utf8_strwidth(how));
 	status_printf_more(s, c, "%s%.*s%s\n", how, len, padding, one);
 	strbuf_release(&onebuf);
 }
@@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
 	what = wt_status_diff_status_string(status);
 	if (!what)
 		BUG("unhandled diff status %c", status);
-	len = label_width - utf8_strwidth(what);
+	len = label_width - cast_size_t_to_int(utf8_strwidth(what));
 	assert(len >= 0);
 	if (one_name != two_name)
 		status_printf_more(s, c, "%s%.*s%s -> %s",

base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
-- 
2.55.0

Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Hardik Kumar]

Changes in v3:
- resolve all signed unsigned warnings.
- cast returns of `utf8_strwidth` where necessary.
- update maxwidth() return type to size_t in wt-status.c
- improve commit message.

Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Phillip Wood]

Hi Hardik

On 27/07/2026 07:59, Hardik Kumar wrote:
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 48d5251..83e4dd6 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
>   					name = ci.author_mail.buf;
>   				else
>   					name = ci.author.buf;
> -				pad = longest_author - utf8_strwidth(name);
> +				pad = longest_author - cast_size_t_to_int(utf8_strwidth(name));
>   				printf(" (%s%*s %10s",
>   				       name, pad, "",
>   				       format_time(ci.author_time,

To me this example perfectly illustrates why changing the return value 
of utf8_strwidth() is a bad idea. The return value is pretty much always 
used to calculate a padding to pass to printf() which expects an int. By 
changing the return value you're forcing all the callers to do the 
conversion themselves which is a bug waiting to happen. I'm also far 
from convinced that the conversions in this patch are complete: grepping 
for 'utf8_strn\{0,1\}width' turns up several calls which do not appear 
to be correctly converted here. For example:

builtin/worktree.c: display[i].width = utf8_strwidth(buf.buf);

where "width" is an int.

I think it would be much better to remove the TODO comment as Junio 
previously suggested and instead add some documentation to the function 
explaining (a) why it is appropriate for it to return an int; (b) why we 
must use the cast_size_t_to_int() helper to prevent overflows (see the 
commit that added that comment).

Thanks

Phillip

> @@ -685,9 +685,9 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
>   			suspect->commit->object.flags |= METAINFO_SHOWN;
>   			get_commit_info(suspect->commit, &ci);
>   			if (*option & OUTPUT_SHOW_EMAIL)
> -				num = utf8_strwidth(ci.author_mail.buf);
> +				num = cast_size_t_to_int(utf8_strwidth(ci.author_mail.buf));
>   			else
> -				num = utf8_strwidth(ci.author.buf);
> +				num = cast_size_t_to_int(utf8_strwidth(ci.author.buf));
>   			if (longest_author < num)
>   				longest_author = num;
>   			commit_info_destroy(&ci);
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 775a797..c4ae95f 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -850,7 +850,7 @@ static void display_ref_update(struct display_state *display_state, char code,
>   			display_state->shown_url = 1;
>   		}
>   
> -		width = (summary_width + strlen(summary) - gettext_width(summary));
> +		width = (summary_width + strlen(summary) - cast_size_t_to_int(gettext_width(summary)));
>   		remote = prettify_refname(remote);
>   		local = prettify_refname(local);
>   
> diff --git a/builtin/repo.c b/builtin/repo.c
> index 84e012f..9c7ad8c 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -371,7 +371,7 @@ static void stats_table_vaddf(struct stats_table *table,
>   
>   	strbuf_vaddf(&buf, format, ap);
>   	formatted_name = strbuf_detach(&buf, NULL);
> -	name_width = utf8_strwidth(formatted_name);
> +	name_width = cast_size_t_to_int(utf8_strwidth(formatted_name));
>   
>   	item = string_list_append_nodup(&table->rows, formatted_name);
>   	item->util = entry;
> @@ -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);
> +		int value_width = cast_size_t_to_int(utf8_strwidth(entry->value));
>   		if (value_width > table->value_col_width)
>   			table->value_col_width = value_width;
>   	}
>   	if (entry->unit) {
> -		int unit_width = utf8_strwidth(entry->unit);
> +		int unit_width = cast_size_t_to_int(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);
> +	int title_name_width = cast_size_t_to_int(utf8_strwidth(name_col_title));
> +	int title_value_width = cast_size_t_to_int(utf8_strwidth(value_col_title));
>   	int name_col_width = table->name_col_width;
>   	int value_col_width = table->value_col_width;
>   	int unit_col_width = table->unit_col_width;
> diff --git a/column.c b/column.c
> index 93fae31..a63d040 100644
> --- a/column.c
> +++ b/column.c
> @@ -26,7 +26,7 @@ struct column_data {
>   /* return length of 's' in letters, ANSI escapes stripped */
>   static int item_length(const char *s)
>   {
> -	return utf8_strnwidth(s, strlen(s), 1);
> +	return cast_size_t_to_int(utf8_strnwidth(s, strlen(s), 1));
>   }
>   
>   /*
> diff --git a/diff.c b/diff.c
> index 589c196..205fedf 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2982,7 +2982,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>   			continue;
>   		}
>   		fill_print_name(file);
> -		len = utf8_strwidth(file->print_name);
> +		len = cast_size_t_to_int(utf8_strwidth(file->print_name));
>   		if (max_len < len)
>   			max_len = len;
>   
> @@ -3037,7 +3037,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>   	 * making the line longer than the maximum width.
>   	 */
>   	if (options->stat_width == -1)
> -		width = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);
> +		width = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));
>   	else
>   		width = options->stat_width ? options->stat_width : 80;
>   	number_width = decimal_width(max_change) > number_width ?
> @@ -3108,7 +3108,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>   		 * "scale" the filename
>   		 */
>   		len = name_width;
> -		name_len = utf8_strwidth(name);
> +		name_len = cast_size_t_to_int(utf8_strwidth(name));
>   		if (name_width < name_len) {
>   			char *slash;
>   			prefix = "...";
> @@ -3123,7 +3123,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;
>   
> 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);
>   #else
>   #define git_gettext_enabled (0)
>   static inline void git_setup_gettext(void)
> diff --git a/pretty.c b/pretty.c
> index d8a9f37..83d4e86 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1809,7 +1809,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>   
>   	if (padding < 0) {
>   		const char *start = strrchr(sb->buf, '\n');
> -		int occupied;
> +		size_t occupied;
>   		if (!start)
>   			start = sb->buf;
>   		occupied = utf8_strnwidth(start, strlen(start), 1);
> @@ -1830,7 +1830,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>   		placeholder++;
>   		total_consumed++;
>   	}
> -	len = utf8_strnwidth(local_sb.buf, local_sb.len, 1);
> +	len = cast_size_t_to_int(utf8_strnwidth(local_sb.buf, local_sb.len, 1));
>   
>   	if (c->flush_type == flush_left_and_steal) {
>   		const char *ch = sb->buf + sb->len - 1;
> diff --git a/utf8.c b/utf8.c
> index 96460cc..cefaefe 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
>    * string, assuming that the string is utf8.  Returns strlen() instead
>    * if the string does not look like a valid utf8 string.
>    */
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>   {
>   	const char *orig = string;
>   	size_t width = 0;
> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
>   		if (glyph_width > 0)
>   			width += glyph_width;
>   	}
> -
> -	/*
> -	 * TODO: fix the interface of this function and `utf8_strwidth()` to
> -	 * return `size_t` instead of `int`.
> -	 */
> -	return cast_size_t_to_int(string ? width : len);
> +	return string ? width : len;
>   }
>   
> -int utf8_strwidth(const char *string)
> +size_t utf8_strwidth(const char *string)
>   {
>   	return utf8_strnwidth(string, strlen(string), 0);
>   }
> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
>   		       const char *s)
>   {
>   	size_t slen = strlen(s);
> -	int display_len = utf8_strnwidth(s, slen, 0);
> +	size_t display_len = utf8_strnwidth(s, slen, 0);
>   	int utf8_compensation = slen - display_len;
>   
>   	if (display_len >= width) {
> diff --git a/utf8.h b/utf8.h
> index cf8ecb0..531e968 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
>   
>   size_t display_mode_esc_sequence_len(const char *s);
>   int utf8_width(const char **start, size_t *remainder_p);
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> -int utf8_strwidth(const char *string);
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> +size_t utf8_strwidth(const char *string);
>   int is_utf8(const char *text);
>   int is_encoding_utf8(const char *name);
>   int same_encoding(const char *, const char *);
> diff --git a/wt-status.c b/wt-status.c
> index 58461e0..672f83b 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -325,13 +325,13 @@ static const char *wt_status_diff_status_string(int status)
>   	}
>   }
>   
> -static int maxwidth(const char *(*label)(int), int minval, int maxval)
> +static size_t maxwidth(const char *(*label)(int), int minval, int maxval)
>   {
>   	int result = 0, i;
>   
>   	for (i = minval; i <= maxval; i++) {
>   		const char *s = label(i);
> -		int len = s ? utf8_strwidth(s) : 0;
> +		size_t len = s ? utf8_strwidth(s) : 0;
>   		if (len > result)
>   			result = len;
>   	}
> @@ -345,7 +345,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,
>   	struct wt_status_change_data *d = it->util;
>   	struct strbuf onebuf = STRBUF_INIT;
>   	static char *padding;
> -	static int label_width;
> +	static size_t label_width;
>   	const char *one, *how;
>   	int len;
>   
> @@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,
>   	status_printf(s, color(WT_STATUS_HEADER, s), "\t");
>   
>   	how = wt_status_unmerged_status_string(d->stagemask);
> -	len = label_width - utf8_strwidth(how);
> +	len = label_width - cast_size_t_to_int(utf8_strwidth(how));
>   	status_printf_more(s, c, "%s%.*s%s\n", how, len, padding, one);
>   	strbuf_release(&onebuf);
>   }
> @@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
>   	what = wt_status_diff_status_string(status);
>   	if (!what)
>   		BUG("unhandled diff status %c", status);
> -	len = label_width - utf8_strwidth(what);
> +	len = label_width - cast_size_t_to_int(utf8_strwidth(what));
>   	assert(len >= 0);
>   	if (one_name != two_name)
>   		status_printf_more(s, c, "%s%.*s%s -> %s",
> 
> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca

Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I think it would be much better to remove the TODO comment as Junio 
> previously suggested and instead add some documentation to the function 
> explaining (a) why it is appropriate for it to return an int; (b) why we 
> must use the cast_size_t_to_int() helper to prevent overflows (see the 
> commit that added that comment).

Thanks, especially for (b) above.  That needs to be stressed if we
are to go in that direction.

Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Hardik Kumar]

On Mon Jul 27, 2026 at 6:21 PM IST, Phillip Wood wrote:

>> diff --git a/builtin/blame.c b/builtin/blame.c
>> index 48d5251..83e4dd6 100644
>> --- a/builtin/blame.c
>> +++ b/builtin/blame.c
>> @@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
>>   					name = ci.author_mail.buf;
>>   				else
>>   					name = ci.author.buf;
>> -				pad = longest_author - utf8_strwidth(name);
>> +				pad = longest_author - cast_size_t_to_int(utf8_strwidth(name));
>>   				printf(" (%s%*s %10s",
>>   				       name, pad, "",
>>   				       format_time(ci.author_time,
>
> To me this example perfectly illustrates why changing the return value 
> of utf8_strwidth() is a bad idea. The return value is pretty much always 
> used to calculate a padding to pass to printf() which expects an int. By 
> changing the return value you're forcing all the callers to do the 
> conversion themselves which is a bug waiting to happen. I'm also far 
> from convinced that the conversions in this patch are complete: grepping 
> for 'utf8_strn\{0,1\}width' turns up several calls which do not appear 
> to be correctly converted here. For example:
>
> builtin/worktree.c: display[i].width = utf8_strwidth(buf.buf);
>
> where "width" is an int.

I had intentionally left out some sites which did not seem could have
any impact by implicit conversions as there are other examples of such
cases where the return value of `strlen` is being assigned to an int
variable. Example:

in combine-diff.c (where len is an int):
	if (len < 0)
		len = strlen(line);

in builtin/update-index.c:
	int namelen = strlen(path);

and other such examples.
>
> I think it would be much better to remove the TODO comment as Junio 
> previously suggested and instead add some documentation to the function 
> explaining (a) why it is appropriate for it to return an int; (b) why we 
> must use the cast_size_t_to_int() helper to prevent overflows (see the 
> commit that added that comment).
This can result in issues down the line and I had mentioned so in a
previous mail but wanted to try it with v3 since I had already been
working on it. I'll send a new patch to remove the TODO. This change
might just not be worth after all.

Thanks,
Hardik

Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Hardik Kumar]

On Mon Jul 27, 2026 at 8:25 PM IST, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> I think it would be much better to remove the TODO comment as Junio 
>> previously suggested and instead add some documentation to the function 
>> explaining (a) why it is appropriate for it to return an int; (b) why we 
>> must use the cast_size_t_to_int() helper to prevent overflows (see the 
>> commit that added that comment).
>
> Thanks, especially for (b) above.  That needs to be stressed if we
> are to go in that direction.

Should this be documented in a new adoc file in the technical
documentation directory?

Thanks,
Hardik

Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t

[Junio C Hamano]

"Hardik Kumar" <hardikxk@gmail.com> writes:

> On Mon Jul 27, 2026 at 8:25 PM IST, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>> I think it would be much better to remove the TODO comment as Junio 
>>> previously suggested and instead add some documentation to the function 
>>> explaining (a) why it is appropriate for it to return an int; (b) why we 
>>> must use the cast_size_t_to_int() helper to prevent overflows (see the 
>>> commit that added that comment).
>>
>> Thanks, especially for (b) above.  That needs to be stressed if we
>> are to go in that direction.
>
> Should this be documented in a new adoc file in the technical
> documentation directory?

The best thing for the new comment to do is to replace the misguided
TODO comment that led us to this exercise.