git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jul 27, 2026, 12:51 UTC
Message-ID
<e971400e-6d23-463f-ae9c-a21d3c5a3563@gmail.com>
In-Reply-To
<20260727065917.469738-1-hardikxk@gmail.com>
Hi Hardik
On 27/07/2026 07:59, Hardik Kumar wrote:
Show 14 quoted lines
> 
> 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
Show 269 quoted lines
> @@ -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
Previous: Hardik KumarNext: Junio C Hamano
Message 14 of 23 in “change utf8_strwidth() return type to size_t”
  1. change utf8_strwidth() return type to size_tHardik Kumar, Jul 26, 2026
  2. René ScharfeJul 26, 2026
  3. Hardik KumarJul 26, 2026
  4. Pablo SabaterJul 26, 2026
  5. Hardik KumarJul 26, 2026
  6. utf8: use size_t for string width methods and callee sites.Hardik Kumar, Jul 26, 2026
  7. Junio C HamanoJul 27, 2026
  8. Junio C HamanoJul 27, 2026
  9. Hardik KumarJul 27, 2026
  10. Pablo SabaterJul 27, 2026
  11. Hardik KumarJul 27, 2026
  12. utf8: make utf8_strwidth() and utf8_strnwidth() return size_tHardik Kumar, Jul 27, 2026
  13. Hardik KumarJul 27, 2026
  14. Phillip WoodJul 27, 2026
  15. Junio C HamanoJul 27, 2026
  16. Hardik KumarJul 27, 2026
  17. Junio C HamanoJul 27, 2026
  18. Hardik KumarJul 27, 2026
  19. utf8: replace utf8_strwidth todo with descriptive commentHardik Kumar, Jul 27, 2026
  20. Phillip WoodJul 28, 2026
  21. Hardik KumarJul 28, 2026
  22. Junio C HamanoJul 28, 2026
  23. utf8: replace utf8_strwidth todo with descriptive commentHardik Kumar, Jul 28, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.