{"thread":{"id":"66068","subject":"[PATCH] change utf8_strwidth() return type to size_t","startedAt":"2026-07-26T12:34:35Z","lastAt":"2026-07-28T18:24:05Z","messageCount":23,"participants":["Hardik Kumar","René Scharfe","Pablo Sabater","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"549011","messageId":"20260726123427.173877-1-hardikxk@gmail.com","threadId":"66068","inReplyTo":null,"subject":"[PATCH] change utf8_strwidth() return type to size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-26T12:34:27Z","receivedAt":"2026-07-26T12:34:35Z","isPatch":true,"body":"The patch changes the return types of `utf8_strwidth()` and\n`utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions\nhave been updated in the header file also.\n\nSigned-off-by: Hardik Kumar \n---\n utf8.c | 13 ++++---------\n utf8.h | 4 ++--\n 2 files changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex 96460cc..1081573 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n * string, assuming that the string is utf8. Returns strlen() instead\n * if the string does not look like a valid utf8 string.\n */\n-int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n {\n \tconst char *orig = string;\n \tsize_t width = 0;\n@@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n \t\tif (glyph_width > 0)\n \t\t\twidth += glyph_width;\n \t}\n-\n-\t/*\n-\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n-\t * return `size_t` instead of `int`.\n-\t */\n-\treturn cast_size_t_to_int(string ? width : len);\n+\treturn (string) ? width : len;\n }\n \n-int utf8_strwidth(const char *string)\n+size_t utf8_strwidth(const char *string)\n {\n \treturn utf8_strnwidth(string, strlen(string), 0);\n }\n@@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n \t\t const char *s)\n {\n \tsize_t slen = strlen(s);\n-\tint display_len = utf8_strnwidth(s, slen, 0);\n+\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n \tint utf8_compensation = slen - display_len;\n \n \tif (display_len >= width) {\ndiff --git a/utf8.h b/utf8.h\nindex cf8ecb0..531e968 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n \n size_t display_mode_esc_sequence_len(const char *s);\n int utf8_width(const char **start, size_t *remainder_p);\n-int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n-int utf8_strwidth(const char *string);\n+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n+size_t utf8_strwidth(const char *string);\n int is_utf8(const char *text);\n int is_encoding_utf8(const char *name);\n int same_encoding(const char *, const char *);\n\nbase-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n-- \n2.55.0\n\n"},{"id":"549012","messageId":"a85b5428-df17-447f-9d84-03fb433711a1@web.de","threadId":"66068","inReplyTo":"20260726123427.173877-1-hardikxk@gmail.com","subject":"Re: [PATCH] change utf8_strwidth() return type to size_t","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-07-26T13:41:33Z","receivedAt":"2026-07-26T13:41:36Z","isPatch":true,"body":"On 7/26/26 2:34 PM, Hardik Kumar wrote:\n> The patch changes the return types of `utf8_strwidth()` and\n> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions\n> have been updated in the header file also.\n> \n> Signed-off-by: Hardik Kumar \n> ---\n> utf8.c | 13 ++++---------\n> utf8.h | 4 ++--\n> 2 files changed, 6 insertions(+), 11 deletions(-)\n\nWhat about callers that still expect int? Are they all safe without\ncast_size_t_to_int()?\n\n> \n> diff --git a/utf8.c b/utf8.c\n> index 96460cc..1081573 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n> * string, assuming that the string is utf8. Returns strlen() instead\n> * if the string does not look like a valid utf8 string.\n> */\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> {\n> \tconst char *orig = string;\n> \tsize_t width = 0;\n> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> \t\tif (glyph_width > 0)\n> \t\t\twidth += glyph_width;\n> \t}\n> -\n> -\t/*\n> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n> -\t * return `size_t` instead of `int`.\n> -\t */\n> -\treturn cast_size_t_to_int(string ? width : len);\n> +\treturn (string) ? width : len;\n\nNit: Why the parentheses around \"string\"?\n\n> }\n> \n> -int utf8_strwidth(const char *string)\n> +size_t utf8_strwidth(const char *string)\n> {\n> \treturn utf8_strnwidth(string, strlen(string), 0);\n> }\n> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n> \t\t const char *s)\n> {\n> \tsize_t slen = strlen(s);\n> -\tint display_len = utf8_strnwidth(s, slen, 0);\n> +\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n> \tint utf8_compensation = slen - display_len;\n> \n> \tif (display_len >= width) {\n> diff --git a/utf8.h b/utf8.h\n> index cf8ecb0..531e968 100644\n> --- a/utf8.h\n> +++ b/utf8.h\n> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n> \n> size_t display_mode_esc_sequence_len(const char *s);\n> int utf8_width(const char **start, size_t *remainder_p);\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> -int utf8_strwidth(const char *string);\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> +size_t utf8_strwidth(const char *string);\n> int is_utf8(const char *text);\n> int is_encoding_utf8(const char *name);\n> int same_encoding(const char *, const char *);\n> \n> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n\n"},{"id":"549013","messageId":"DK8L6JM14UNS.16B15DIOFW1K5@gmail.com","threadId":"66068","inReplyTo":"20260726123427.173877-1-hardikxk@gmail.com","subject":"Re: [PATCH] change utf8_strwidth() return type to size_t","fromName":"Pablo Sabater","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-07-26T14:52:34Z","receivedAt":"2026-07-26T14:52:38Z","isPatch":true,"body":"On Sun Jul 26, 2026 at 2:34 PM CEST, Hardik Kumar wrote:\n> The patch changes the return types of `utf8_strwidth()` and\n\nRegarding the presentation: \"The patch changes...\", try to avoid this\npattern, I think something like this would fit better:\n\nutf8_strwidth() and utf8_strnwidth() return int, even though the value\nthey return is always non-negative:\n\n- utf8_strnwidth() accumulates the width into a size_t and otherwise\n returns its size_t len parameter,\n- utf8_strwidth() just forwards its result.\n\nChange their signatures to return size_t instead.\n\nIf you want to mention the TODO, I would add it after the '---'.\n\n> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions\n> have been updated in the header file also.\n>\n> Signed-off-by: Hardik Kumar \n> ---\n> utf8.c | 13 ++++---------\n> utf8.h | 4 ++--\n> 2 files changed, 6 insertions(+), 11 deletions(-)\n>\n> diff --git a/utf8.c b/utf8.c\n> index 96460cc..1081573 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n> * string, assuming that the string is utf8. Returns strlen() instead\n> * if the string does not look like a valid utf8 string.\n> */\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> {\n> \tconst char *orig = string;\n> \tsize_t width = 0;\n> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> \t\tif (glyph_width > 0)\n> \t\t\twidth += glyph_width;\n> \t}\n> -\n> -\t/*\n> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n> -\t * return `size_t` instead of `int`.\n> -\t */\n> -\treturn cast_size_t_to_int(string ? width : len);\n> +\treturn (string) ? width : len;\n\nnit: parentheses at \"(string)\" are unnecessary.\n\nAlso, cast_size_t_to_int() had an overflow check, we need to be sure\nthat no caller relies on that check. If you have checked for that,\nplease mention it in the commit message.\n\n> }\n>\n> -int utf8_strwidth(const char *string)\n> +size_t utf8_strwidth(const char *string)\n> {\n> \treturn utf8_strnwidth(string, strlen(string), 0);\n> }\n> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n> \t\t const char *s)\n> {\n> \tsize_t slen = strlen(s);\n> -\tint display_len = utf8_strnwidth(s, slen, 0);\n> +\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n\nWe are fixing a caller here and that is correct.\nBut these functions that we've changed in this patch are called\nthroughout the codebase, we should fix those callers too.\n\nWe can check who their callers are with:\n\n git grep -n -E 'utf8_str.?width'\n\nbuiltin/repo.c:390: int value_width = utf8_strwidth(entry->value);\nbuiltin/repo.c:395: int unit_width = utf8_strwidth(entry->unit);\nbuiltin/repo.c:585: int title_name_width = utf8_strwidth(name_col_title);\nbuiltin/repo.c:586: int title_value_width = utf8_strwidth(value_col_title);\n\n(there are more)\n\nFrom what I reviewed, no caller will break because of this, but I think\nwe should fix it for consistency.\n\n> \tint utf8_compensation = slen - display_len;\n>\n> \tif (display_len >= width) {\n> diff --git a/utf8.h b/utf8.h\n> index cf8ecb0..531e968 100644\n> --- a/utf8.h\n> +++ b/utf8.h\n> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n>\n> size_t display_mode_esc_sequence_len(const char *s);\n> int utf8_width(const char **start, size_t *remainder_p);\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> -int utf8_strwidth(const char *string);\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> +size_t utf8_strwidth(const char *string);\n> int is_utf8(const char *text);\n> int is_encoding_utf8(const char *name);\n> int same_encoding(const char *, const char *);\n>\n> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n\nThe signature change looks ok.\n\nRegards,\nPablo\n\n"},{"id":"549022","messageId":"DK8MEZUFXK0Q.RTW35IRY7R4@gmail.com","threadId":"66068","inReplyTo":"a85b5428-df17-447f-9d84-03fb433711a1@web.de","subject":"Re: [PATCH] change utf8_strwidth() return type to size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-26T15:50:37Z","receivedAt":"2026-07-26T15:50:43Z","isPatch":true,"body":"On Sun Jul 26, 2026 at 7:11 PM IST, René Scharfe wrote:\n> On 7/26/26 2:34 PM, Hardik Kumar wrote:\n>> The patch changes the return types of `utf8_strwidth()` and\n>> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions\n>> have been updated in the header file also.\n>> \n>> Signed-off-by: Hardik Kumar \n>> ---\n>> utf8.c | 13 ++++---------\n>> utf8.h | 4 ++--\n>> 2 files changed, 6 insertions(+), 11 deletions(-)\n>\n> What about callers that still expect int? Are they all safe without\n> cast_size_t_to_int()?\n>\nThe return type should be implicitly converted back to int for all the\nlocations its being called at. If implicit conversions are not\nencouraged I could change the types of the variables at the call sites?\n\n>> \n>> diff --git a/utf8.c b/utf8.c\n>> index 96460cc..1081573 100644\n>> --- a/utf8.c\n>> +++ b/utf8.c\n>> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n>> * string, assuming that the string is utf8. Returns strlen() instead\n>> * if the string does not look like a valid utf8 string.\n>> */\n>> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n>> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n>> {\n>> \tconst char *orig = string;\n>> \tsize_t width = 0;\n>> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n>> \t\tif (glyph_width > 0)\n>> \t\t\twidth += glyph_width;\n>> \t}\n>> -\n>> -\t/*\n>> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n>> -\t * return `size_t` instead of `int`.\n>> -\t */\n>> -\treturn cast_size_t_to_int(string ? width : len);\n>> +\treturn (string) ? width : len;\n>\n> Nit: Why the parentheses around \"string\"?\n>\nBad habit I'll drop them in v2. Makes it obvious we are expecting a bool\nvalue here.\n\n>> }\n>> \n>> -int utf8_strwidth(const char *string)\n>> +size_t utf8_strwidth(const char *string)\n>> {\n>> \treturn utf8_strnwidth(string, strlen(string), 0);\n>> }\n>> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n>> \t\t const char *s)\n>> {\n>> \tsize_t slen = strlen(s);\n>> -\tint display_len = utf8_strnwidth(s, slen, 0);\n>> +\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n>> \tint utf8_compensation = slen - display_len;\n>> \n>> \tif (display_len >= width) {\n>> diff --git a/utf8.h b/utf8.h\n>> index cf8ecb0..531e968 100644\n>> --- a/utf8.h\n>> +++ b/utf8.h\n>> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n>> \n>> size_t display_mode_esc_sequence_len(const char *s);\n>> int utf8_width(const char **start, size_t *remainder_p);\n>> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n>> -int utf8_strwidth(const char *string);\n>> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n>> +size_t utf8_strwidth(const char *string);\n>> int is_utf8(const char *text);\n>> int is_encoding_utf8(const char *name);\n>> int same_encoding(const char *, const char *);\n>> \n>> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n\n"},{"id":"549023","messageId":"DK8MG5RUM3WT.1KGOZVF7WWILF@gmail.com","threadId":"66068","inReplyTo":"DK8L6JM14UNS.16B15DIOFW1K5@gmail.com","subject":"Re: [PATCH] change utf8_strwidth() return type to size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-26T15:52:09Z","receivedAt":"2026-07-26T15:52:13Z","isPatch":true,"body":"Noted! I'll write up as suggested in v2 for this patch.\n"},{"id":"549039","messageId":"20260726195718.1914131-1-hardikxk@gmail.com","threadId":"66068","inReplyTo":"DK8L6JM14UNS.16B15DIOFW1K5@gmail.com","subject":"[PATCH v2] utf8: use size_t for string width methods and callee sites.","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-26T19:57:18Z","receivedAt":"2026-07-26T19:57:32Z","isPatch":true,"body":"utf8_strwidth() and utf8_strnwidth() return int, even though the\nreturn value is always non-negative:\n\n- utf8_strnwidth() accumulates the width into a size_t and otherwise\n returns its size_t len parameter,\n- utf8_strwidth() just forwards its result.\n\nChange their signatures to return size_t instead.\n\nUpdate the types of the variables the said method is used to avoid\npotential UB caused by implicit conversion from size_t to int.\n\nThe returned values from `utf8_strwidth()` are casted to int at places\nwhere it was falling tests or required other changes.\n\nSigned-off-by: Hardik Kumar \n---\nChanges in v2:\n- reworked types for utf8_strwidth and its sites of usage.\n- removed redundant parens around `string`.\n- updated commit message for better explaining the patch.\n\n builtin/blame.c | 4 ++--\n builtin/branch.c | 2 +-\n builtin/repo.c | 10 +++++-----\n column.c | 2 +-\n diff.c | 7 ++++---\n gettext.c | 2 +-\n gettext.h | 2 +-\n pretty.c | 5 +++--\n utf8.c | 13 ++++---------\n utf8.h | 4 ++--\n wt-status.c | 8 ++++----\n 11 files changed, 28 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 48d5251..2d24b63 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t\t\tname = ci.author_mail.buf;\n \t\t\t\telse\n \t\t\t\t\tname = ci.author.buf;\n-\t\t\t\tpad = longest_author - utf8_strwidth(name);\n+\t\t\t\tpad = longest_author - cast_size_t_to_int(utf8_strwidth(name));\n \t\t\t\tprintf(\" (%s%*s %10s\",\n \t\t\t\t name, pad, \"\",\n \t\t\t\t format_time(ci.author_time,\n@@ -668,7 +668,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)\n \n \tfor (e = sb->ent; e; e = e->next) {\n \t\tstruct blame_origin *suspect = e->suspect;\n-\t\tint num;\n+\t\tsize_t num;\n \t\tsize_t marks_count = count_marks(e, *option);\n \n \t\tif (max_marks_count < marks_count)\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex dede60d..514ba64 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -354,7 +354,7 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n \tfor (i = 0; i < refs->nr; i++) {\n \t\tstruct ref_array_item *it = refs->items[i];\n \t\tconst char *desc = it->refname;\n-\t\tint w;\n+\t\tsize_t w;\n \n \t\tskip_prefix(it->refname, \"refs/heads/\", &desc);\n \t\tskip_prefix(it->refname, \"refs/remotes/\", &desc);\ndiff --git a/builtin/repo.c b/builtin/repo.c\nindex 84e012f..47b9191 100644\n--- a/builtin/repo.c\n+++ b/builtin/repo.c\n@@ -367,7 +367,7 @@ static void stats_table_vaddf(struct stats_table *table,\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list_item *item;\n \tchar *formatted_name;\n-\tint name_width;\n+\tsize_t name_width;\n \n \tstrbuf_vaddf(&buf, format, ap);\n \tformatted_name = strbuf_detach(&buf, NULL);\n@@ -387,12 +387,12 @@ static void stats_table_vaddf(struct stats_table *table,\n \t\tstring_list_append_nodup(&table->annotations, strbuf_detach(&buf, NULL));\n \t}\n \tif (entry->value) {\n-\t\tint value_width = utf8_strwidth(entry->value);\n+\t\tsize_t value_width = utf8_strwidth(entry->value);\n \t\tif (value_width > table->value_col_width)\n \t\t\ttable->value_col_width = value_width;\n \t}\n \tif (entry->unit) {\n-\t\tint unit_width = utf8_strwidth(entry->unit);\n+\t\tsize_t unit_width = utf8_strwidth(entry->unit);\n \t\tif (unit_width > table->unit_col_width)\n \t\t\ttable->unit_col_width = unit_width;\n \t}\n@@ -582,8 +582,8 @@ static void stats_table_print_structure(const struct stats_table *table)\n {\n \tconst char *name_col_title = _(\"Repository structure\");\n \tconst char *value_col_title = _(\"Value\");\n-\tint title_name_width = utf8_strwidth(name_col_title);\n-\tint title_value_width = utf8_strwidth(value_col_title);\n+\tsize_t title_name_width = utf8_strwidth(name_col_title);\n+\tsize_t title_value_width = utf8_strwidth(value_col_title);\n \tint name_col_width = table->name_col_width;\n \tint value_col_width = table->value_col_width;\n \tint unit_col_width = table->unit_col_width;\ndiff --git a/column.c b/column.c\nindex 93fae31..6b7f921 100644\n--- a/column.c\n+++ b/column.c\n@@ -24,7 +24,7 @@ struct column_data {\n };\n \n /* return length of 's' in letters, ANSI escapes stripped */\n-static int item_length(const char *s)\n+static size_t item_length(const char *s)\n {\n \treturn utf8_strnwidth(s, strlen(s), 1);\n }\ndiff --git a/diff.c b/diff.c\nindex 589c196..4887958 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2952,7 +2952,8 @@ static int utf8_ish_width(const char **start)\n \n static void show_stats(struct diffstat_t *data, struct diff_options *options)\n {\n-\tint i, len, add, del, adds = 0, dels = 0;\n+\tint i, add, del, adds = 0, dels = 0;\n+\tsize_t len;\n \tuintmax_t max_change = 0, max_len = 0;\n \tint total_files = data->nr, count;\n \tint width, name_width, graph_width, number_width = 0, bin_width = 0;\n@@ -3037,7 +3038,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t * making the line longer than the maximum width.\n \t */\n \tif (options->stat_width == -1)\n-\t\twidth = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);\n+\t\twidth = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));\n \telse\n \t\twidth = options->stat_width ? options->stat_width : 80;\n \tnumber_width = decimal_width(max_change) > number_width ?\n@@ -3123,7 +3124,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tif (slash)\n \t\t\t\tname = slash;\n \t\t}\n-\t\tpadding = len - utf8_strwidth(name);\n+\t\tpadding = len - cast_size_t_to_int(utf8_strwidth(name));\n \t\tif (padding < 0)\n \t\t\tpadding = 0;\n \ndiff --git a/gettext.c b/gettext.c\nindex 8d08a61..4d5d05e 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -129,7 +129,7 @@ void git_setup_gettext(void)\n }\n \n /* return the number of columns of string 's' in current locale */\n-int gettext_width(const char *s)\n+size_t gettext_width(const char *s)\n {\n \tstatic int is_utf8 = -1;\n \tif (is_utf8 == -1)\ndiff --git a/gettext.h b/gettext.h\nindex 484cafa..f161a21 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -31,7 +31,7 @@\n #ifndef NO_GETTEXT\n extern int git_gettext_enabled;\n void git_setup_gettext(void);\n-int gettext_width(const char *s);\n+size_t gettext_width(const char *s);\n #else\n #define git_gettext_enabled (0)\n static inline void git_setup_gettext(void)\ndiff --git a/pretty.c b/pretty.c\nindex d8a9f37..f7d392d 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1805,11 +1805,12 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n {\n \tstruct strbuf local_sb = STRBUF_INIT;\n \tsize_t total_consumed = 0;\n-\tint len, padding = c->padding;\n+\tint padding = c->padding;\n+\tsize_t len;\n \n \tif (padding < 0) {\n \t\tconst char *start = strrchr(sb->buf, '\\n');\n-\t\tint occupied;\n+\t\tsize_t occupied;\n \t\tif (!start)\n \t\t\tstart = sb->buf;\n \t\toccupied = utf8_strnwidth(start, strlen(start), 1);\ndiff --git a/utf8.c b/utf8.c\nindex 96460cc..cefaefe 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n * string, assuming that the string is utf8. Returns strlen() instead\n * if the string does not look like a valid utf8 string.\n */\n-int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n {\n \tconst char *orig = string;\n \tsize_t width = 0;\n@@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n \t\tif (glyph_width > 0)\n \t\t\twidth += glyph_width;\n \t}\n-\n-\t/*\n-\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n-\t * return `size_t` instead of `int`.\n-\t */\n-\treturn cast_size_t_to_int(string ? width : len);\n+\treturn string ? width : len;\n }\n \n-int utf8_strwidth(const char *string)\n+size_t utf8_strwidth(const char *string)\n {\n \treturn utf8_strnwidth(string, strlen(string), 0);\n }\n@@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n \t\t const char *s)\n {\n \tsize_t slen = strlen(s);\n-\tint display_len = utf8_strnwidth(s, slen, 0);\n+\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n \tint utf8_compensation = slen - display_len;\n \n \tif (display_len >= width) {\ndiff --git a/utf8.h b/utf8.h\nindex cf8ecb0..531e968 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n \n size_t display_mode_esc_sequence_len(const char *s);\n int utf8_width(const char **start, size_t *remainder_p);\n-int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n-int utf8_strwidth(const char *string);\n+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n+size_t utf8_strwidth(const char *string);\n int is_utf8(const char *text);\n int is_encoding_utf8(const char *name);\n int same_encoding(const char *, const char *);\ndiff --git a/wt-status.c b/wt-status.c\nindex 58461e0..0e1e32d 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -331,9 +331,9 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)\n \n \tfor (i = minval; i <= maxval; i++) {\n \t\tconst char *s = label(i);\n-\t\tint len = s ? utf8_strwidth(s) : 0;\n+\t\tsize_t len = s ? utf8_strwidth(s) : 0;\n \t\tif (len > result)\n-\t\t\tresult = len;\n+\t\t\tresult = cast_size_t_to_int(len);\n \t}\n \treturn result;\n }\n@@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \n \thow = wt_status_unmerged_status_string(d->stagemask);\n-\tlen = label_width - utf8_strwidth(how);\n+\tlen = label_width - cast_size_t_to_int(utf8_strwidth(how));\n \tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n \tstrbuf_release(&onebuf);\n }\n@@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \twhat = wt_status_diff_status_string(status);\n \tif (!what)\n \t\tBUG(\"unhandled diff status %c\", status);\n-\tlen = label_width - utf8_strwidth(what);\n+\tlen = label_width - cast_size_t_to_int(utf8_strwidth(what));\n \tassert(len >= 0);\n \tif (one_name != two_name)\n \t\tstatus_printf_more(s, c, \"%s%.*s%s -> %s\",\n-- \n2.55.0\n\n"},{"id":"549052","messageId":"xmqqpl09s3cc.fsf@gitster.g","threadId":"66068","inReplyTo":"20260726195718.1914131-1-hardikxk@gmail.com","subject":"Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-27T00:06:43Z","receivedAt":"2026-07-27T00:06:46Z","isPatch":true,"body":"Hardik Kumar writes:\n\n> utf8_strwidth() and utf8_strnwidth() return int, even though the\n> return value is always non-negative:\n>\n> - utf8_strnwidth() accumulates the width into a size_t and otherwise\n> returns its size_t len parameter,\n> - utf8_strwidth() just forwards its result.\n>\n> Change their signatures to return size_t instead.\n>\n> Update the types of the variables the said method is used to avoid\n> potential UB caused by implicit conversion from size_t to int.\n\nThe goal looks attractive on the surface, and the change to make\nutf8_strwidth() and utf8_strnwidth() return 'size_t' clears an\nexisting TODO. However, the updates to the call sites to support\nthis change introduce several bugs due to unsigned integer underflow\nand incorrect mixed-sign comparisons.\n\nConsider just one example:\n\n> diff --git a/diff.c b/diff.c\n> index 589c196..4887958 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2952,7 +2952,8 @@ static int utf8_ish_width(const char **start)\n> \n> static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> {\n> -\tint i, len, add, del, adds = 0, dels = 0;\n> +\tint i, add, del, adds = 0, dels = 0;\n> +\tsize_t len;\n> \tuintmax_t max_change = 0, max_len = 0;\n> \tint total_files = data->nr, count;\n> \tint width, name_width, graph_width, number_width = 0, bin_width = 0;\n\nThe above change impacts code later in the function (among other\nthings):\n\n\t\t/*\n\t\t * \"scale\" the filename\n\t\t */\n\t\tlen = name_width;\n\t\tname_len = utf8_strwidth(name);\n\t\tif (name_width < name_len) {\n\t\t\tchar *slash;\n\t\t\tprefix = \"...\";\n\t\t\tlen -= 3;\n\t\t\tif (len < 0)\n\t\t\t\tlen = 0;\n\nHere, 'len' used to be an 'int', but now it is 'size_t', which is\nunsigned. The safeguard to prevent 'len' from going down to an\nunacceptably low value by clipping it to 0 never triggers, because\n'if (len < 0)' can never be true. If len is less than 3, len -= 3\nwill result in a fairly large value, and the subsequent computation\nwould go bananas to see a value with little relation to name_len.\n\nAnother example.\n\n> diff --git a/pretty.c b/pretty.c\n> index d8a9f37..f7d392d 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1805,11 +1805,12 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n> {\n> \tstruct strbuf local_sb = STRBUF_INIT;\n> \tsize_t total_consumed = 0;\n> -\tint len, padding = c->padding;\n> +\tint padding = c->padding;\n> +\tsize_t len;\n> \n> \tif (padding < 0) {\n> \t\tconst char *start = strrchr(sb->buf, '\\n');\n> -\t\tint occupied;\n> +\t\tsize_t occupied;\n> \t\tif (!start)\n> \t\t\tstart = sb->buf;\n> \t\toccupied = utf8_strnwidth(start, strlen(start), 1);\n\nAfter this post-context, 'occupied' is incremented, and then we have\nthis:\n\n\t\tpadding = (-padding) - occupied;\n\nIf 'occupied' is sufficiently large, 'padding' can become negative\nhere. Because padding remains an 'int' and can become negative, it\nimpacts code a bit further down in the same function (among other\nsimilar comparisons):\n\n\tif (c->flush_type == flush_left_and_steal) {\n\t\tconst char *ch = sb->buf + sb->len - 1;\n\t\twhile (len > padding && ch > sb->buf) {\n\t\t\tconst char *p;\n\t\t\tif (*ch == ' ') {\n\t\t\t\tch--;\n\t\t\t\tpadding++;\n\t\t\t\tcontinue;\n\t\t\t}\n\nWe compare 'len' and 'padding', first promoting 'padding' to\n'size_t', so when 'padding' is negative, we compare 'len' with a\nfairly large number due to unsigned wraparound. We will fail to\n\"steal\" spaces as we will not loop here.\n\nI will stop here. What makes reviewing this change so unpleasant is\nthat on the surface, changing variable definitions to flip int to\nsize_t looks pretty, yet the real breakage appears in places that\nare not shown in the patch at all.\n\nSo, this needs more work to become acceptable, I am afraid.\n"},{"id":"549061","messageId":"DK8Y8F4650AW.1XN921ROZW70F@gmail.com","threadId":"66068","inReplyTo":"20260726195718.1914131-1-hardikxk@gmail.com","subject":"Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.","fromName":"Pablo Sabater","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-07-27T01:06:15Z","receivedAt":"2026-07-27T01:06:20Z","isPatch":true,"body":"[+cc Junio, who reviewed this while I was writing mine, to keep him in\nthe thread]\n\nHi!\n\nNote that you have sent v2 in-reply-to my review from last version, not\nto the v1.\n\nThis Patch does not compile with DEVELOPER=1:\n\n $ make DEVELOPER=1\n\n builtin/blame.c:681:20: error: comparison of integers of different signs: 'int' and 'size_t'\n (aka 'unsigned long') [-Werror,-Wsign-compare]\n 681 | if (longest_file < num)\n | ~~~~~~~~~~~~ ^ ~~~\n builtin/blame.c:691:23: error: comparison of integers of different signs: 'int' and 'size_t'\n (aka 'unsigned long') [-Werror,-Wsign-compare]\n 691 | if (longest_author < num)\n | ~~~~~~~~~~~~~~ ^ ~~~\n builtin/blame.c:696:25: error: comparison of integers of different signs: 'int' and 'size_t'\n (aka 'unsigned long') [-Werror,-Wsign-compare]\n 696 | if (longest_src_lines < num)\n | ~~~~~~~~~~~~~~~~~ ^ ~~~\n builtin/blame.c:699:25: error: comparison of integers of different signs: 'int' and 'size_t'\n (aka 'unsigned long') [-Werror,-Wsign-compare]\n 699 | if (longest_dst_lines < num)\n | ~~~~~~~~~~~~~~~~~ ^ ~~~\n\nAlso note that many files have DISABLE_SIGN_COMPARE_WARNINGS which hides\nfrom us this errors.\n\nThat's why we have to be extra careful when changing signatures of\nfunctions accross the codebase.\n\nThe title is being too explicit, when a function signature changes, it\nis expected that their callers will change, no need to add it to the\ntitle. What about:\n\n utf8: make utf8_strwidth() and utf8_strnwidth() return size_t\n\ndoes it work?\n\nOn Sun Jul 26, 2026 at 9:57 PM CEST, Hardik Kumar wrote:\n> utf8_strwidth() and utf8_strnwidth() return int, even though the\n> return value is always non-negative:\n>\n> - utf8_strnwidth() accumulates the width into a size_t and otherwise\n> returns its size_t len parameter,\n\nnit: change the comma for a dot.\n\n> - utf8_strwidth() just forwards its result.\n>\n> Change their signatures to return size_t instead.\n>\n> Update the types of the variables the said method is used to avoid\n> potential UB caused by implicit conversion from size_t to int.\n\nThis is not correct and it reads a bit off, what about:\n\n Update the types of the variables where these functions are used, to\n avoid the implicit conversion from size_t to int.\n\nThis is not correct because the implicit conversion from size_t to\nint is not undefined behavior.\n\n>\n> The returned values from `utf8_strwidth()` are casted to int at places\n> where it was falling tests or required other changes.\n\nnit: s/casted/cast/\nnit: s/falling/failing/\n\n>\n> Signed-off-by: Hardik Kumar \n> ---\n> Changes in v2:\n> - reworked types for utf8_strwidth and its sites of usage.\n> - removed redundant parens around `string`.\n> - updated commit message for better explaining the patch.\n>\n> builtin/blame.c | 4 ++--\n> builtin/branch.c | 2 +-\n> builtin/repo.c | 10 +++++-----\n> column.c | 2 +-\n> diff.c | 7 ++++---\n> gettext.c | 2 +-\n> gettext.h | 2 +-\n> pretty.c | 5 +++--\n> utf8.c | 13 ++++---------\n> utf8.h | 4 ++--\n> wt-status.c | 8 ++++----\n> 11 files changed, 28 insertions(+), 31 deletions(-)\n>\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 48d5251..2d24b63 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t\t\t\t\tname = ci.author_mail.buf;\n> \t\t\t\telse\n> \t\t\t\t\tname = ci.author.buf;\n> -\t\t\t\tpad = longest_author - utf8_strwidth(name);\n> +\t\t\t\tpad = longest_author - cast_size_t_to_int(utf8_strwidth(name));\n\nThis one is fine.\n\n> \t\t\t\tprintf(\" (%s%*s %10s\",\n> \t\t\t\t name, pad, \"\",\n> \t\t\t\t format_time(ci.author_time,\n> @@ -668,7 +668,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)\n>\n> \tfor (e = sb->ent; e; e = e->next) {\n> \t\tstruct blame_origin *suspect = e->suspect;\n> -\t\tint num;\n> +\t\tsize_t num;\n\nLooking at how num is used, it is reused for multiple things:\n- strlen()\n- utf8_strwidth()\n- line-number sums\n\nThe longest_* variables we compare num against are still int.\n\nCan we split num into different variables?\n\n> \t\tsize_t marks_count = count_marks(e, *option);\n>\n> \t\tif (max_marks_count < marks_count)\n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index dede60d..514ba64 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -354,7 +354,7 @@ static int calc_maxwidth(struct ref_array *refs, int remote_bonus)\n> \tfor (i = 0; i < refs->nr; i++) {\n> \t\tstruct ref_array_item *it = refs->items[i];\n> \t\tconst char *desc = it->refname;\n> -\t\tint w;\n> +\t\tsize_t w;\n\nw receives utf8_strwidth() but later we have:\n\n if (w > max)\n\nThis is now size_t > int.\nHere I would keep w int and cast.\n\n>\n> \t\tskip_prefix(it->refname, \"refs/heads/\", &desc);\n> \t\tskip_prefix(it->refname, \"refs/remotes/\", &desc);\n> diff --git a/builtin/repo.c b/builtin/repo.c\n> index 84e012f..47b9191 100644\n> --- a/builtin/repo.c\n> +++ b/builtin/repo.c\n> @@ -367,7 +367,7 @@ static void stats_table_vaddf(struct stats_table *table,\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tstruct string_list_item *item;\n> \tchar *formatted_name;\n> -\tint name_width;\n> +\tsize_t name_width;\n\nSame as above:\n\n if (name_width > table->name_col_width)\n\nI think that these three fields can be promoted safely\n\n struct stats_table {\n\t [snip]\n\n\t int name_col_width;\n\t int value_col_width;\n\t int unit_col_width;\n };\n\nbut check every use of them afterwards for code that still expects an\nint.\n\n>\n> \tstrbuf_vaddf(&buf, format, ap);\n> \tformatted_name = strbuf_detach(&buf, NULL);\n> @@ -387,12 +387,12 @@ static void stats_table_vaddf(struct stats_table *table,\n> \t\tstring_list_append_nodup(&table->annotations, strbuf_detach(&buf, NULL));\n> \t}\n> \tif (entry->value) {\n> -\t\tint value_width = utf8_strwidth(entry->value);\n> +\t\tsize_t value_width = utf8_strwidth(entry->value);\n\nI feel this one is partially my fault, I wrote these as example output\nof the grep I sent last reroll. But they still need to be checked:\n\n> \t\tif (value_width > table->value_col_width)\n\nWe are comparing size_t > int.\n\n> \t\t\ttable->value_col_width = value_width;\n\nWe are narrowing size_t to int.\n\n> \t}\n> \tif (entry->unit) {\n> -\t\tint unit_width = utf8_strwidth(entry->unit);\n> +\t\tsize_t unit_width = utf8_strwidth(entry->unit);\n> \t\tif (unit_width > table->unit_col_width)\n> \t\t\ttable->unit_col_width = unit_width;\n> \t}\n> @@ -582,8 +582,8 @@ static void stats_table_print_structure(const struct stats_table *table)\n> {\n> \tconst char *name_col_title = _(\"Repository structure\");\n> \tconst char *value_col_title = _(\"Value\");\n> -\tint title_name_width = utf8_strwidth(name_col_title);\n> -\tint title_value_width = utf8_strwidth(value_col_title);\n> +\tsize_t title_name_width = utf8_strwidth(name_col_title);\n> +\tsize_t title_value_width = utf8_strwidth(value_col_title);\n\nSame problem, these are compared against int *_col_width locals,\nand:\n value_col_width = title_value_width - unit_col_width\n\nbelow the context now mixes size_t and int. Promoting the struct fields\nas suggested above fixes all of this at once.\n\n\n> \tint name_col_width = table->name_col_width;\n> \tint value_col_width = table->value_col_width;\n> \tint unit_col_width = table->unit_col_width;\n> diff --git a/column.c b/column.c\n> index 93fae31..6b7f921 100644\n> --- a/column.c\n> +++ b/column.c\n> @@ -24,7 +24,7 @@ struct column_data {\n> };\n>\n> /* return length of 's' in letters, ANSI escapes stripped */\n> -static int item_length(const char *s)\n> +static size_t item_length(const char *s)\n> {\n> \treturn utf8_strnwidth(s, strlen(s), 1);\n> }\n\nitem_length() has only one caller, which stores the result into an\nint *, so the value gets narrowed right back to int and this change\nbuys nothing. Keep returning int and cast inside.\n\n> diff --git a/diff.c b/diff.c\n> index 589c196..4887958 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2952,7 +2952,8 @@ static int utf8_ish_width(const char **start)\n>\n> static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> {\n> -\tint i, len, add, del, adds = 0, dels = 0;\n> +\tint i, add, del, adds = 0, dels = 0;\n> +\tsize_t len;\n\nThis will have problems below:\n\n\t[snip]\n\n\tlen = name_width;\n\tname_len = utf8_strwidth(name);\n\tif (name_width < name_len) {\n\t\tprefix = \"...\";\n\t\tlen -= 3;\n\t\tif (len < 0)\n\t\t\tlen = 0;\n\t\twhile (name_len > len && *name)\n\n\t[snip]\n\nif (len < 0) will always be false for a size_t, becoming dead code.\nAlso name_len > len is int > size_t.\n\nLet's step back a bit.\nname_len receives utf8_strwidth(), let's make it size_t too, and\ncompare against len so the types stay consistent. The dead code can\nbecome:\n\n\tlen = len > 3 ? len - 3 : 0;\n\nWe would also have to change the check:\n\n\tif (name_width < name_len)\nto:\n\tif (len < name_len)\n\n> \tuintmax_t max_change = 0, max_len = 0;\n> \tint total_files = data->nr, count;\n> \tint width, name_width, graph_width, number_width = 0, bin_width = 0;\n> @@ -3037,7 +3038,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t * making the line longer than the maximum width.\n> \t */\n> \tif (options->stat_width == -1)\n> -\t\twidth = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);\n> +\t\twidth = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));\n\nThis one is fine.\n\n> \telse\n> \t\twidth = options->stat_width ? options->stat_width : 80;\n> \tnumber_width = decimal_width(max_change) > number_width ?\n> @@ -3123,7 +3124,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\t\tif (slash)\n> \t\t\t\tname = slash;\n> \t\t}\n> -\t\tpadding = len - utf8_strwidth(name);\n> +\t\tpadding = len - cast_size_t_to_int(utf8_strwidth(name));\n> \t\tif (padding < 0)\n> \t\t\tpadding = 0;\n\nThe cast doesn't work here because len is also size_t. We could do this\nto be sure that there will be no problems:\n\n\tsize_t name_disp = utf8_strwidth(name);\n\tif (name_disp > len)\n\t\tpadding = 0;\n\telse\n\t\tpadding = cast_size_t_to_int(len - name_disp);\n\n>\n> diff --git a/gettext.c b/gettext.c\n> index 8d08a61..4d5d05e 100644\n> --- a/gettext.c\n> +++ b/gettext.c\n> @@ -129,7 +129,7 @@ void git_setup_gettext(void)\n> }\n>\n> /* return the number of columns of string 's' in current locale */\n> -int gettext_width(const char *s)\n> +size_t gettext_width(const char *s)\n> {\n> \tstatic int is_utf8 = -1;\n> \tif (is_utf8 == -1)\n> diff --git a/gettext.h b/gettext.h\n> index 484cafa..f161a21 100644\n> --- a/gettext.h\n> +++ b/gettext.h\n> @@ -31,7 +31,7 @@\n> #ifndef NO_GETTEXT\n> extern int git_gettext_enabled;\n> void git_setup_gettext(void);\n> -int gettext_width(const char *s);\n> +size_t gettext_width(const char *s);\n\nCareful, this is inside an #ifndef, if we change the signature here,\nthe other branch must follow.\n\n> #else\n> #define git_gettext_enabled (0)\n> static inline void git_setup_gettext(void)\n> diff --git a/pretty.c b/pretty.c\n> index d8a9f37..f7d392d 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1805,11 +1805,12 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n> {\n> \tstruct strbuf local_sb = STRBUF_INIT;\n> \tsize_t total_consumed = 0;\n> -\tint len, padding = c->padding;\n> +\tint padding = c->padding;\n> +\tsize_t len;\n>\n> \tif (padding < 0) {\n> \t\tconst char *start = strrchr(sb->buf, '\\n');\n> -\t\tint occupied;\n> +\t\tsize_t occupied;\n> \t\tif (!start)\n> \t\t\tstart = sb->buf;\n> \t\toccupied = utf8_strnwidth(start, strlen(start), 1);\n\npadding is signed on purpose, so we can't have len as size_t.\nLater we have len > padding.\nKeep len and occupied as int, casting the utf8_strnwidth() results\ninstead.\n\n> diff --git a/utf8.c b/utf8.c\n> index 96460cc..cefaefe 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n> * string, assuming that the string is utf8. Returns strlen() instead\n> * if the string does not look like a valid utf8 string.\n> */\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> {\n> \tconst char *orig = string;\n> \tsize_t width = 0;\n> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> \t\tif (glyph_width > 0)\n> \t\t\twidth += glyph_width;\n> \t}\n> -\n> -\t/*\n> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n> -\t * return `size_t` instead of `int`.\n> -\t */\n> -\treturn cast_size_t_to_int(string ? width : len);\n> +\treturn string ? width : len;\n> }\n\nThis is good.\n\n>\n> -int utf8_strwidth(const char *string)\n> +size_t utf8_strwidth(const char *string)\n> {\n> \treturn utf8_strnwidth(string, strlen(string), 0);\n> }\n> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n> \t\t const char *s)\n> {\n> \tsize_t slen = strlen(s);\n> -\tint display_len = utf8_strnwidth(s, slen, 0);\n> +\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n> \tint utf8_compensation = slen - display_len;\n\nThis is fine.\n\n>\n> \tif (display_len >= width) {\n> diff --git a/utf8.h b/utf8.h\n> index cf8ecb0..531e968 100644\n> --- a/utf8.h\n> +++ b/utf8.h\n> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n>\n> size_t display_mode_esc_sequence_len(const char *s);\n> int utf8_width(const char **start, size_t *remainder_p);\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> -int utf8_strwidth(const char *string);\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> +size_t utf8_strwidth(const char *string);\n> int is_utf8(const char *text);\n> int is_encoding_utf8(const char *name);\n> int same_encoding(const char *, const char *);\n> diff --git a/wt-status.c b/wt-status.c\n> index 58461e0..0e1e32d 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -331,9 +331,9 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)\n>\n> \tfor (i = minval; i <= maxval; i++) {\n> \t\tconst char *s = label(i);\n> -\t\tint len = s ? utf8_strwidth(s) : 0;\n> +\t\tsize_t len = s ? utf8_strwidth(s) : 0;\n> \t\tif (len > result)\n> -\t\t\tresult = len;\n> +\t\t\tresult = cast_size_t_to_int(len);\n> \t}\n> \treturn result;\n> }\n\nThis function returns non-negative, I would say that the TODO applies to\nit as well because it returns 0 or utf8_strwidth(). Cleaning it to return\nsize_t would help in some steps below:\n\n> @@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n> \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n>\n> \thow = wt_status_unmerged_status_string(d->stagemask);\n> -\tlen = label_width - utf8_strwidth(how);\n> +\tlen = label_width - cast_size_t_to_int(utf8_strwidth(how));\n> \tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n> \tstrbuf_release(&onebuf);\n> }\n> @@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n> \twhat = wt_status_diff_status_string(status);\n> \tif (!what)\n> \t\tBUG(\"unhandled diff status %c\", status);\n> -\tlen = label_width - utf8_strwidth(what);\n> +\tlen = label_width - cast_size_t_to_int(utf8_strwidth(what));\n> \tassert(len >= 0);\n\nWe can see here that len has to be non-negative. label_width comes from\nmaxwidth(). With the cleanup I suggested, maxwidth() returns size_t and\nso does label_width.\n\nWe can have the same safety with:\n\n\tsize_t what_width = utf8_strwidth(what);\n\n\tif (what_width > label_width)\n\t\tBUG(\"label wider than column\");\n\tlen = cast_size_t_to_int(label_width - what_width);\n\nYou need the cast at the end anyway because len is used as the\nprecision in \"%.*s\".\n\nBut I think this way leaves the code cleaner.\n\n> \tif (one_name != two_name)\n> \t\tstatus_printf_more(s, c, \"%s%.*s%s -> %s\",\n\nPlease know that the hunks of code I suggest might not be the direct\nsolution, so don't take them directly. I do this because I don't want\nto write whole functions and I think that if I just write what's\nrelevant it will be easier to understand.\n\nAlso, part of your work as author is to verify what you add and be able\nto defend it, which means taking what others say (including this\nreview) with a grain of salt, everyone can make mistakes. You can run\nthe tests on your own to check before sending the next version.\n\nNice work,\nPablo\n"},{"id":"549063","messageId":"xmqqbjbtqdv4.fsf@gitster.g","threadId":"66068","inReplyTo":"xmqqpl09s3cc.fsf@gitster.g","subject":"Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-27T04:02:23Z","receivedAt":"2026-07-27T04:02:26Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> The goal looks attractive on the surface, and the change to make\n> utf8_strwidth() and utf8_strnwidth() return 'size_t' clears an\n> existing TODO. However, the updates to the call sites to support\n> this change introduce several bugs due to unsigned integer underflow\n> and incorrect mixed-sign comparisons.\n\nHaving said that, we need to remember that these two functions are\nnot designed for anything more than what fits on a single line. The\nonly reason they exist in our codebase is because their callers want\nto measure the display width of a string, so that they can align\nelements on a line vertically with the corresponding elements on the\nprevious and next lines.\n\nThis does not mean we do not need to support more than 80 columns\n;-), but they surely do not have to support a 2-billion-column-wide\ndisplay.\n\nQuite honestly, I have to say that this topic has a very low\nexpected benefit in practice, while it costs us quite a lot by\nhaving to carefully code and even more carefully review. If we have\nto endure so many new bugs in the callers just to clear an existing\nTODO, we might be better off not doing so and relying on the \"safe\ncast from size_t down to int that barfs if the quantity does not fit\nin an int\" protection.\n"},{"id":"549067","messageId":"DK95C0SCPDX3.28ORSCO088KJ9@gmail.com","threadId":"66068","inReplyTo":"DK8Y8F4650AW.1XN921ROZW70F@gmail.com","subject":"Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T06:40:05Z","receivedAt":"2026-07-27T06:40:12Z","isPatch":true,"body":"On Mon Jul 27, 2026 at 6:36 AM IST, Pablo Sabater wrote:\n\n>> \t\t\t\tprintf(\" (%s%*s %10s\",\n>> \t\t\t\t name, pad, \"\",\n>> \t\t\t\t format_time(ci.author_time,\n>> @@ -668,7 +668,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)\n>>\n>> \tfor (e = sb->ent; e; e = e->next) {\n>> \t\tstruct blame_origin *suspect = e->suspect;\n>> -\t\tint num;\n>> +\t\tsize_t num;\n>\n> Looking at how num is used, it is reused for multiple things:\n> - strlen()\n> - utf8_strwidth()\n> - line-number sums\n>\n> The longest_* variables we compare num against are still int.\n>\n> Can we split num into different variables?\n>\nI think it would be better to just cast the return of `utf8_strwidth` to\nint instead when assigning it num.\n\n>>\n>> \t\tskip_prefix(it->refname, \"refs/heads/\", &desc);\n>> \t\tskip_prefix(it->refname, \"refs/remotes/\", &desc);\n>> diff --git a/builtin/repo.c b/builtin/repo.c\n>> index 84e012f..47b9191 100644\n>> --- a/builtin/repo.c\n>> +++ b/builtin/repo.c\n>> @@ -367,7 +367,7 @@ static void stats_table_vaddf(struct stats_table *table,\n>> \tstruct strbuf buf = STRBUF_INIT;\n>> \tstruct string_list_item *item;\n>> \tchar *formatted_name;\n>> -\tint name_width;\n>> +\tsize_t name_width;\n\n> Same as above:\n>\n> if (name_width > table->name_col_width)\n>\n> I think that these three fields can be promoted safely\n>\n> struct stats_table {\n> \t [snip]\n>\n> \t int name_col_width;\n> \t int value_col_width;\n> \t int unit_col_width;\n> };\n>\n> but check every use of them afterwards for code that still expects an\n> int.\nChanges to the struct field types might generate more signed unsigned\nwarnings leading to changes to fix things which might just not be\nnecessary for this. There probably won't be a use case requiring a very\nhigh number for col_width.\n\n>>\n>> \tstrbuf_vaddf(&buf, format, ap);\n>> \tformatted_name = strbuf_detach(&buf, NULL);\n>> @@ -387,12 +387,12 @@ static void stats_table_vaddf(struct stats_table *table,\n>> \t\tstring_list_append_nodup(&table->annotations, strbuf_detach(&buf, NULL));\n>> \t}\n>> \tif (entry->value) {\n>> -\t\tint value_width = utf8_strwidth(entry->value);\n>> +\t\tsize_t value_width = utf8_strwidth(entry->value);\n>\n> I feel this one is partially my fault, I wrote these as example output\n> of the grep I sent last reroll. But they still need to be checked:\n>\n>> \t\tif (value_width > table->value_col_width)\n>\n> We are comparing size_t > int.\n>\n>> \t\t\ttable->value_col_width = value_width;\n>\n> We are narrowing size_t to int.\n>\n>> \t}\n>> \tif (entry->unit) {\n>> -\t\tint unit_width = utf8_strwidth(entry->unit);\n>> +\t\tsize_t unit_width = utf8_strwidth(entry->unit);\n>> \t\tif (unit_width > table->unit_col_width)\n>> \t\t\ttable->unit_col_width = unit_width;\n>> \t}\n>> @@ -582,8 +582,8 @@ static void stats_table_print_structure(const struct stats_table *table)\n>> {\n>> \tconst char *name_col_title = _(\"Repository structure\");\n>> \tconst char *value_col_title = _(\"Value\");\n>> -\tint title_name_width = utf8_strwidth(name_col_title);\n>> -\tint title_value_width = utf8_strwidth(value_col_title);\n>> +\tsize_t title_name_width = utf8_strwidth(name_col_title);\n>> +\tsize_t title_value_width = utf8_strwidth(value_col_title);\n>\n> Same problem, these are compared against int *_col_width locals,\n> and:\n> value_col_width = title_value_width - unit_col_width\n>\n> below the context now mixes size_t and int. Promoting the struct fields\n> as suggested above fixes all of this at once.\nOtherwise keeping these as int like before and casting the returns where\nnecessary also works in this case.\n\n>> \telse\n>> \t\twidth = options->stat_width ? options->stat_width : 80;\n>> \tnumber_width = decimal_width(max_change) > number_width ?\n>> @@ -3123,7 +3124,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n>> \t\t\tif (slash)\n>> \t\t\t\tname = slash;\n>> \t\t}\n>> -\t\tpadding = len - utf8_strwidth(name);\n>> +\t\tpadding = len - cast_size_t_to_int(utf8_strwidth(name));\n>> \t\tif (padding < 0)\n>> \t\t\tpadding = 0;\n>\n> The cast doesn't work here because len is also size_t. We could do this\n> to be sure that there will be no problems:\n>\n> \tsize_t name_disp = utf8_strwidth(name);\n> \tif (name_disp > len)\n> \t\tpadding = 0;\n> \telse\n> \t\tpadding = cast_size_t_to_int(len - name_disp);\n>\nChanging len back to int resolves this as well as the previous one.\n\n>>\n>> diff --git a/gettext.c b/gettext.c\n>> index 8d08a61..4d5d05e 100644\n>> --- a/gettext.c\n>> +++ b/gettext.c\n>> @@ -129,7 +129,7 @@ void git_setup_gettext(void)\n>> }\n>>\n>> /* return the number of columns of string 's' in current locale */\n>> -int gettext_width(const char *s)\n>> +size_t gettext_width(const char *s)\n>> {\n>> \tstatic int is_utf8 = -1;\n>> \tif (is_utf8 == -1)\n>> diff --git a/gettext.h b/gettext.h\n>> index 484cafa..f161a21 100644\n>> --- a/gettext.h\n>> +++ b/gettext.h\n>> @@ -31,7 +31,7 @@\n>> #ifndef NO_GETTEXT\n>> extern int git_gettext_enabled;\n>> void git_setup_gettext(void);\n>> -int gettext_width(const char *s);\n>> +size_t gettext_width(const char *s);\n>\n> Careful, this is inside an #ifndef, if we change the signature here,\n> the other branch must follow.\nThe implmentations returns either a `utf8_strwidth()` or `strlent()`\nboth of which would return a `size_t`. The function is only called in a\nsingle place so I suppose casting it back to int where its called would\nbe better.\n\nThanks,\nHardik.\n"},{"id":"549068","messageId":"DK95E6MN2LYU.3P2KB11V2SAS7@gmail.com","threadId":"66068","inReplyTo":"xmqqbjbtqdv4.fsf@gitster.g","subject":"Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T06:42:55Z","receivedAt":"2026-07-27T06:43:01Z","isPatch":true,"body":"On Mon Jul 27, 2026 at 9:32 AM IST, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> Quite honestly, I have to say that this topic has a very low\n> expected benefit in practice, while it costs us quite a lot by\n> having to carefully code and even more carefully review. If we have\n> to endure so many new bugs in the callers just to clear an existing\n> TODO, we might be better off not doing so and relying on the \"safe\n> cast from size_t down to int that barfs if the quantity does not fit\n> in an int\" protection.\n\nI agree that while this might not net something significant and yes\ngoing through this is difficult but, many places it would much rather\nmake sense having an unsigned int as mostly its rare that we would be\ndealing with negatives except in a few cases which you highlighted\nbefore and others I got when reworking.\n\nI would like to send up a patch with some better changes done. I had\npreviously not built and tested with warnings enabled apologies for\nthat I assumed the defaults to enable them without explicit args.\n\nThanks,\nHardik\n"},{"id":"549070","messageId":"20260727065917.469738-1-hardikxk@gmail.com","threadId":"66068","inReplyTo":"20260726123427.173877-1-hardikxk@gmail.com","subject":"[PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T06:59:17Z","receivedAt":"2026-07-27T06:59:24Z","isPatch":true,"body":"utf8_strwidth() and utf8_strnwidth() return int, even though the\nreturn value is always non-negative:\n\n- utf8_strnwidth() accumulates the width into a size_t and otherwise\n returns its size_t len parameter.\n- utf8_strwidth() just forwards its result.\n\nChange their signatures to return size_t instead.\n\nUpdate the types of the variables where these method is used to avoid\nimplicit conversion from size_t to int.\n\nThe return values from `utf8_strwidth()` are cast to int where\nnegative values are expected or depend on other int variables.\n\nSigned-off-by: Hardik Kumar \n---\n builtin/blame.c | 6 +++---\n builtin/fetch.c | 2 +-\n builtin/repo.c | 10 +++++-----\n column.c | 2 +-\n diff.c | 8 ++++----\n gettext.c | 2 +-\n gettext.h | 2 +-\n pretty.c | 4 ++--\n utf8.c | 13 ++++---------\n utf8.h | 4 ++--\n wt-status.c | 10 +++++-----\n 11 files changed, 29 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 48d5251..83e4dd6 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,\n \t\t\t\t\tname = ci.author_mail.buf;\n \t\t\t\telse\n \t\t\t\t\tname = ci.author.buf;\n-\t\t\t\tpad = longest_author - utf8_strwidth(name);\n+\t\t\t\tpad = longest_author - cast_size_t_to_int(utf8_strwidth(name));\n \t\t\t\tprintf(\" (%s%*s %10s\",\n \t\t\t\t name, pad, \"\",\n \t\t\t\t format_time(ci.author_time,\n@@ -685,9 +685,9 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)\n \t\t\tsuspect->commit->object.flags |= METAINFO_SHOWN;\n \t\t\tget_commit_info(suspect->commit, &ci);\n \t\t\tif (*option & OUTPUT_SHOW_EMAIL)\n-\t\t\t\tnum = utf8_strwidth(ci.author_mail.buf);\n+\t\t\t\tnum = cast_size_t_to_int(utf8_strwidth(ci.author_mail.buf));\n \t\t\telse\n-\t\t\t\tnum = utf8_strwidth(ci.author.buf);\n+\t\t\t\tnum = cast_size_t_to_int(utf8_strwidth(ci.author.buf));\n \t\t\tif (longest_author < num)\n \t\t\t\tlongest_author = num;\n \t\t\tcommit_info_destroy(&ci);\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 775a797..c4ae95f 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -850,7 +850,7 @@ static void display_ref_update(struct display_state *display_state, char code,\n \t\t\tdisplay_state->shown_url = 1;\n \t\t}\n \n-\t\twidth = (summary_width + strlen(summary) - gettext_width(summary));\n+\t\twidth = (summary_width + strlen(summary) - cast_size_t_to_int(gettext_width(summary)));\n \t\tremote = prettify_refname(remote);\n \t\tlocal = prettify_refname(local);\n \ndiff --git a/builtin/repo.c b/builtin/repo.c\nindex 84e012f..9c7ad8c 100644\n--- a/builtin/repo.c\n+++ b/builtin/repo.c\n@@ -371,7 +371,7 @@ static void stats_table_vaddf(struct stats_table *table,\n \n \tstrbuf_vaddf(&buf, format, ap);\n \tformatted_name = strbuf_detach(&buf, NULL);\n-\tname_width = utf8_strwidth(formatted_name);\n+\tname_width = cast_size_t_to_int(utf8_strwidth(formatted_name));\n \n \titem = string_list_append_nodup(&table->rows, formatted_name);\n \titem->util = entry;\n@@ -387,12 +387,12 @@ static void stats_table_vaddf(struct stats_table *table,\n \t\tstring_list_append_nodup(&table->annotations, strbuf_detach(&buf, NULL));\n \t}\n \tif (entry->value) {\n-\t\tint value_width = utf8_strwidth(entry->value);\n+\t\tint value_width = cast_size_t_to_int(utf8_strwidth(entry->value));\n \t\tif (value_width > table->value_col_width)\n \t\t\ttable->value_col_width = value_width;\n \t}\n \tif (entry->unit) {\n-\t\tint unit_width = utf8_strwidth(entry->unit);\n+\t\tint unit_width = cast_size_t_to_int(utf8_strwidth(entry->unit));\n \t\tif (unit_width > table->unit_col_width)\n \t\t\ttable->unit_col_width = unit_width;\n \t}\n@@ -582,8 +582,8 @@ static void stats_table_print_structure(const struct stats_table *table)\n {\n \tconst char *name_col_title = _(\"Repository structure\");\n \tconst char *value_col_title = _(\"Value\");\n-\tint title_name_width = utf8_strwidth(name_col_title);\n-\tint title_value_width = utf8_strwidth(value_col_title);\n+\tint title_name_width = cast_size_t_to_int(utf8_strwidth(name_col_title));\n+\tint title_value_width = cast_size_t_to_int(utf8_strwidth(value_col_title));\n \tint name_col_width = table->name_col_width;\n \tint value_col_width = table->value_col_width;\n \tint unit_col_width = table->unit_col_width;\ndiff --git a/column.c b/column.c\nindex 93fae31..a63d040 100644\n--- a/column.c\n+++ b/column.c\n@@ -26,7 +26,7 @@ struct column_data {\n /* return length of 's' in letters, ANSI escapes stripped */\n static int item_length(const char *s)\n {\n-\treturn utf8_strnwidth(s, strlen(s), 1);\n+\treturn cast_size_t_to_int(utf8_strnwidth(s, strlen(s), 1));\n }\n \n /*\ndiff --git a/diff.c b/diff.c\nindex 589c196..205fedf 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2982,7 +2982,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tcontinue;\n \t\t}\n \t\tfill_print_name(file);\n-\t\tlen = utf8_strwidth(file->print_name);\n+\t\tlen = cast_size_t_to_int(utf8_strwidth(file->print_name));\n \t\tif (max_len < len)\n \t\t\tmax_len = len;\n \n@@ -3037,7 +3037,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t * making the line longer than the maximum width.\n \t */\n \tif (options->stat_width == -1)\n-\t\twidth = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);\n+\t\twidth = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));\n \telse\n \t\twidth = options->stat_width ? options->stat_width : 80;\n \tnumber_width = decimal_width(max_change) > number_width ?\n@@ -3108,7 +3108,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t * \"scale\" the filename\n \t\t */\n \t\tlen = name_width;\n-\t\tname_len = utf8_strwidth(name);\n+\t\tname_len = cast_size_t_to_int(utf8_strwidth(name));\n \t\tif (name_width < name_len) {\n \t\t\tchar *slash;\n \t\t\tprefix = \"...\";\n@@ -3123,7 +3123,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n \t\t\tif (slash)\n \t\t\t\tname = slash;\n \t\t}\n-\t\tpadding = len - utf8_strwidth(name);\n+\t\tpadding = len - cast_size_t_to_int(utf8_strwidth(name));\n \t\tif (padding < 0)\n \t\t\tpadding = 0;\n \ndiff --git a/gettext.c b/gettext.c\nindex 8d08a61..4d5d05e 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -129,7 +129,7 @@ void git_setup_gettext(void)\n }\n \n /* return the number of columns of string 's' in current locale */\n-int gettext_width(const char *s)\n+size_t gettext_width(const char *s)\n {\n \tstatic int is_utf8 = -1;\n \tif (is_utf8 == -1)\ndiff --git a/gettext.h b/gettext.h\nindex 484cafa..f161a21 100644\n--- a/gettext.h\n+++ b/gettext.h\n@@ -31,7 +31,7 @@\n #ifndef NO_GETTEXT\n extern int git_gettext_enabled;\n void git_setup_gettext(void);\n-int gettext_width(const char *s);\n+size_t gettext_width(const char *s);\n #else\n #define git_gettext_enabled (0)\n static inline void git_setup_gettext(void)\ndiff --git a/pretty.c b/pretty.c\nindex d8a9f37..83d4e86 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1809,7 +1809,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n \n \tif (padding < 0) {\n \t\tconst char *start = strrchr(sb->buf, '\\n');\n-\t\tint occupied;\n+\t\tsize_t occupied;\n \t\tif (!start)\n \t\t\tstart = sb->buf;\n \t\toccupied = utf8_strnwidth(start, strlen(start), 1);\n@@ -1830,7 +1830,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n \t\tplaceholder++;\n \t\ttotal_consumed++;\n \t}\n-\tlen = utf8_strnwidth(local_sb.buf, local_sb.len, 1);\n+\tlen = cast_size_t_to_int(utf8_strnwidth(local_sb.buf, local_sb.len, 1));\n \n \tif (c->flush_type == flush_left_and_steal) {\n \t\tconst char *ch = sb->buf + sb->len - 1;\ndiff --git a/utf8.c b/utf8.c\nindex 96460cc..cefaefe 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n * string, assuming that the string is utf8. Returns strlen() instead\n * if the string does not look like a valid utf8 string.\n */\n-int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n {\n \tconst char *orig = string;\n \tsize_t width = 0;\n@@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n \t\tif (glyph_width > 0)\n \t\t\twidth += glyph_width;\n \t}\n-\n-\t/*\n-\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n-\t * return `size_t` instead of `int`.\n-\t */\n-\treturn cast_size_t_to_int(string ? width : len);\n+\treturn string ? width : len;\n }\n \n-int utf8_strwidth(const char *string)\n+size_t utf8_strwidth(const char *string)\n {\n \treturn utf8_strnwidth(string, strlen(string), 0);\n }\n@@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n \t\t const char *s)\n {\n \tsize_t slen = strlen(s);\n-\tint display_len = utf8_strnwidth(s, slen, 0);\n+\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n \tint utf8_compensation = slen - display_len;\n \n \tif (display_len >= width) {\ndiff --git a/utf8.h b/utf8.h\nindex cf8ecb0..531e968 100644\n--- a/utf8.h\n+++ b/utf8.h\n@@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n \n size_t display_mode_esc_sequence_len(const char *s);\n int utf8_width(const char **start, size_t *remainder_p);\n-int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n-int utf8_strwidth(const char *string);\n+size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n+size_t utf8_strwidth(const char *string);\n int is_utf8(const char *text);\n int is_encoding_utf8(const char *name);\n int same_encoding(const char *, const char *);\ndiff --git a/wt-status.c b/wt-status.c\nindex 58461e0..672f83b 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -325,13 +325,13 @@ static const char *wt_status_diff_status_string(int status)\n \t}\n }\n \n-static int maxwidth(const char *(*label)(int), int minval, int maxval)\n+static size_t maxwidth(const char *(*label)(int), int minval, int maxval)\n {\n \tint result = 0, i;\n \n \tfor (i = minval; i <= maxval; i++) {\n \t\tconst char *s = label(i);\n-\t\tint len = s ? utf8_strwidth(s) : 0;\n+\t\tsize_t len = s ? utf8_strwidth(s) : 0;\n \t\tif (len > result)\n \t\t\tresult = len;\n \t}\n@@ -345,7 +345,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstruct wt_status_change_data *d = it->util;\n \tstruct strbuf onebuf = STRBUF_INIT;\n \tstatic char *padding;\n-\tstatic int label_width;\n+\tstatic size_t label_width;\n \tconst char *one, *how;\n \tint len;\n \n@@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \n \thow = wt_status_unmerged_status_string(d->stagemask);\n-\tlen = label_width - utf8_strwidth(how);\n+\tlen = label_width - cast_size_t_to_int(utf8_strwidth(how));\n \tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n \tstrbuf_release(&onebuf);\n }\n@@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \twhat = wt_status_diff_status_string(status);\n \tif (!what)\n \t\tBUG(\"unhandled diff status %c\", status);\n-\tlen = label_width - utf8_strwidth(what);\n+\tlen = label_width - cast_size_t_to_int(utf8_strwidth(what));\n \tassert(len >= 0);\n \tif (one_name != two_name)\n \t\tstatus_printf_more(s, c, \"%s%.*s%s -> %s\",\n\nbase-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n-- \n2.55.0\n\n"},{"id":"549071","messageId":"DK95URJHRRGR.88DI7EGK5OMO@gmail.com","threadId":"66068","inReplyTo":"20260727065917.469738-1-hardikxk@gmail.com","subject":"Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T07:04:34Z","receivedAt":"2026-07-27T07:04:41Z","isPatch":true,"body":"Changes in v3:\n- resolve all signed unsigned warnings.\n- cast returns of `utf8_strwidth` where necessary.\n- update maxwidth() return type to size_t in wt-status.c\n- improve commit message.\n"},{"id":"549083","messageId":"e971400e-6d23-463f-ae9c-a21d3c5a3563@gmail.com","threadId":"66068","inReplyTo":"20260727065917.469738-1-hardikxk@gmail.com","subject":"Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-27T12:51:27Z","receivedAt":"2026-07-27T12:51:21Z","isPatch":true,"body":"Hi Hardik\n\nOn 27/07/2026 07:59, Hardik Kumar wrote:\n> \n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 48d5251..83e4dd6 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,\n> \t\t\t\t\tname = ci.author_mail.buf;\n> \t\t\t\telse\n> \t\t\t\t\tname = ci.author.buf;\n> -\t\t\t\tpad = longest_author - utf8_strwidth(name);\n> +\t\t\t\tpad = longest_author - cast_size_t_to_int(utf8_strwidth(name));\n> \t\t\t\tprintf(\" (%s%*s %10s\",\n> \t\t\t\t name, pad, \"\",\n> \t\t\t\t format_time(ci.author_time,\n\nTo me this example perfectly illustrates why changing the return value \nof utf8_strwidth() is a bad idea. The return value is pretty much always \nused to calculate a padding to pass to printf() which expects an int. By \nchanging the return value you're forcing all the callers to do the \nconversion themselves which is a bug waiting to happen. I'm also far \nfrom convinced that the conversions in this patch are complete: grepping \nfor 'utf8_strn\\{0,1\\}width' turns up several calls which do not appear \nto be correctly converted here. For example:\n\nbuiltin/worktree.c: display[i].width = utf8_strwidth(buf.buf);\n\nwhere \"width\" is an int.\n\nI think it would be much better to remove the TODO comment as Junio \npreviously suggested and instead add some documentation to the function \nexplaining (a) why it is appropriate for it to return an int; (b) why we \nmust use the cast_size_t_to_int() helper to prevent overflows (see the \ncommit that added that comment).\n\nThanks\n\nPhillip\n\n> @@ -685,9 +685,9 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)\n> \t\t\tsuspect->commit->object.flags |= METAINFO_SHOWN;\n> \t\t\tget_commit_info(suspect->commit, &ci);\n> \t\t\tif (*option & OUTPUT_SHOW_EMAIL)\n> -\t\t\t\tnum = utf8_strwidth(ci.author_mail.buf);\n> +\t\t\t\tnum = cast_size_t_to_int(utf8_strwidth(ci.author_mail.buf));\n> \t\t\telse\n> -\t\t\t\tnum = utf8_strwidth(ci.author.buf);\n> +\t\t\t\tnum = cast_size_t_to_int(utf8_strwidth(ci.author.buf));\n> \t\t\tif (longest_author < num)\n> \t\t\t\tlongest_author = num;\n> \t\t\tcommit_info_destroy(&ci);\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 775a797..c4ae95f 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -850,7 +850,7 @@ static void display_ref_update(struct display_state *display_state, char code,\n> \t\t\tdisplay_state->shown_url = 1;\n> \t\t}\n> \n> -\t\twidth = (summary_width + strlen(summary) - gettext_width(summary));\n> +\t\twidth = (summary_width + strlen(summary) - cast_size_t_to_int(gettext_width(summary)));\n> \t\tremote = prettify_refname(remote);\n> \t\tlocal = prettify_refname(local);\n> \n> diff --git a/builtin/repo.c b/builtin/repo.c\n> index 84e012f..9c7ad8c 100644\n> --- a/builtin/repo.c\n> +++ b/builtin/repo.c\n> @@ -371,7 +371,7 @@ static void stats_table_vaddf(struct stats_table *table,\n> \n> \tstrbuf_vaddf(&buf, format, ap);\n> \tformatted_name = strbuf_detach(&buf, NULL);\n> -\tname_width = utf8_strwidth(formatted_name);\n> +\tname_width = cast_size_t_to_int(utf8_strwidth(formatted_name));\n> \n> \titem = string_list_append_nodup(&table->rows, formatted_name);\n> \titem->util = entry;\n> @@ -387,12 +387,12 @@ static void stats_table_vaddf(struct stats_table *table,\n> \t\tstring_list_append_nodup(&table->annotations, strbuf_detach(&buf, NULL));\n> \t}\n> \tif (entry->value) {\n> -\t\tint value_width = utf8_strwidth(entry->value);\n> +\t\tint value_width = cast_size_t_to_int(utf8_strwidth(entry->value));\n> \t\tif (value_width > table->value_col_width)\n> \t\t\ttable->value_col_width = value_width;\n> \t}\n> \tif (entry->unit) {\n> -\t\tint unit_width = utf8_strwidth(entry->unit);\n> +\t\tint unit_width = cast_size_t_to_int(utf8_strwidth(entry->unit));\n> \t\tif (unit_width > table->unit_col_width)\n> \t\t\ttable->unit_col_width = unit_width;\n> \t}\n> @@ -582,8 +582,8 @@ static void stats_table_print_structure(const struct stats_table *table)\n> {\n> \tconst char *name_col_title = _(\"Repository structure\");\n> \tconst char *value_col_title = _(\"Value\");\n> -\tint title_name_width = utf8_strwidth(name_col_title);\n> -\tint title_value_width = utf8_strwidth(value_col_title);\n> +\tint title_name_width = cast_size_t_to_int(utf8_strwidth(name_col_title));\n> +\tint title_value_width = cast_size_t_to_int(utf8_strwidth(value_col_title));\n> \tint name_col_width = table->name_col_width;\n> \tint value_col_width = table->value_col_width;\n> \tint unit_col_width = table->unit_col_width;\n> diff --git a/column.c b/column.c\n> index 93fae31..a63d040 100644\n> --- a/column.c\n> +++ b/column.c\n> @@ -26,7 +26,7 @@ struct column_data {\n> /* return length of 's' in letters, ANSI escapes stripped */\n> static int item_length(const char *s)\n> {\n> -\treturn utf8_strnwidth(s, strlen(s), 1);\n> +\treturn cast_size_t_to_int(utf8_strnwidth(s, strlen(s), 1));\n> }\n> \n> /*\n> diff --git a/diff.c b/diff.c\n> index 589c196..205fedf 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2982,7 +2982,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\t\tcontinue;\n> \t\t}\n> \t\tfill_print_name(file);\n> -\t\tlen = utf8_strwidth(file->print_name);\n> +\t\tlen = cast_size_t_to_int(utf8_strwidth(file->print_name));\n> \t\tif (max_len < len)\n> \t\t\tmax_len = len;\n> \n> @@ -3037,7 +3037,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t * making the line longer than the maximum width.\n> \t */\n> \tif (options->stat_width == -1)\n> -\t\twidth = term_columns() - utf8_strnwidth(line_prefix, strlen(line_prefix), 1);\n> +\t\twidth = term_columns() - cast_size_t_to_int(utf8_strnwidth(line_prefix, strlen(line_prefix), 1));\n> \telse\n> \t\twidth = options->stat_width ? options->stat_width : 80;\n> \tnumber_width = decimal_width(max_change) > number_width ?\n> @@ -3108,7 +3108,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\t * \"scale\" the filename\n> \t\t */\n> \t\tlen = name_width;\n> -\t\tname_len = utf8_strwidth(name);\n> +\t\tname_len = cast_size_t_to_int(utf8_strwidth(name));\n> \t\tif (name_width < name_len) {\n> \t\t\tchar *slash;\n> \t\t\tprefix = \"...\";\n> @@ -3123,7 +3123,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)\n> \t\t\tif (slash)\n> \t\t\t\tname = slash;\n> \t\t}\n> -\t\tpadding = len - utf8_strwidth(name);\n> +\t\tpadding = len - cast_size_t_to_int(utf8_strwidth(name));\n> \t\tif (padding < 0)\n> \t\t\tpadding = 0;\n> \n> diff --git a/gettext.c b/gettext.c\n> index 8d08a61..4d5d05e 100644\n> --- a/gettext.c\n> +++ b/gettext.c\n> @@ -129,7 +129,7 @@ void git_setup_gettext(void)\n> }\n> \n> /* return the number of columns of string 's' in current locale */\n> -int gettext_width(const char *s)\n> +size_t gettext_width(const char *s)\n> {\n> \tstatic int is_utf8 = -1;\n> \tif (is_utf8 == -1)\n> diff --git a/gettext.h b/gettext.h\n> index 484cafa..f161a21 100644\n> --- a/gettext.h\n> +++ b/gettext.h\n> @@ -31,7 +31,7 @@\n> #ifndef NO_GETTEXT\n> extern int git_gettext_enabled;\n> void git_setup_gettext(void);\n> -int gettext_width(const char *s);\n> +size_t gettext_width(const char *s);\n> #else\n> #define git_gettext_enabled (0)\n> static inline void git_setup_gettext(void)\n> diff --git a/pretty.c b/pretty.c\n> index d8a9f37..83d4e86 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1809,7 +1809,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n> \n> \tif (padding < 0) {\n> \t\tconst char *start = strrchr(sb->buf, '\\n');\n> -\t\tint occupied;\n> +\t\tsize_t occupied;\n> \t\tif (!start)\n> \t\t\tstart = sb->buf;\n> \t\toccupied = utf8_strnwidth(start, strlen(start), 1);\n> @@ -1830,7 +1830,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n> \t\tplaceholder++;\n> \t\ttotal_consumed++;\n> \t}\n> -\tlen = utf8_strnwidth(local_sb.buf, local_sb.len, 1);\n> +\tlen = cast_size_t_to_int(utf8_strnwidth(local_sb.buf, local_sb.len, 1));\n> \n> \tif (c->flush_type == flush_left_and_steal) {\n> \t\tconst char *ch = sb->buf + sb->len - 1;\n> diff --git a/utf8.c b/utf8.c\n> index 96460cc..cefaefe 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)\n> * string, assuming that the string is utf8. Returns strlen() instead\n> * if the string does not look like a valid utf8 string.\n> */\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> {\n> \tconst char *orig = string;\n> \tsize_t width = 0;\n> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> \t\tif (glyph_width > 0)\n> \t\t\twidth += glyph_width;\n> \t}\n> -\n> -\t/*\n> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n> -\t * return `size_t` instead of `int`.\n> -\t */\n> -\treturn cast_size_t_to_int(string ? width : len);\n> +\treturn string ? width : len;\n> }\n> \n> -int utf8_strwidth(const char *string)\n> +size_t utf8_strwidth(const char *string)\n> {\n> \treturn utf8_strnwidth(string, strlen(string), 0);\n> }\n> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid\n> \t\t const char *s)\n> {\n> \tsize_t slen = strlen(s);\n> -\tint display_len = utf8_strnwidth(s, slen, 0);\n> +\tsize_t display_len = utf8_strnwidth(s, slen, 0);\n> \tint utf8_compensation = slen - display_len;\n> \n> \tif (display_len >= width) {\n> diff --git a/utf8.h b/utf8.h\n> index cf8ecb0..531e968 100644\n> --- a/utf8.h\n> +++ b/utf8.h\n> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */\n> \n> size_t display_mode_esc_sequence_len(const char *s);\n> int utf8_width(const char **start, size_t *remainder_p);\n> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> -int utf8_strwidth(const char *string);\n> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);\n> +size_t utf8_strwidth(const char *string);\n> int is_utf8(const char *text);\n> int is_encoding_utf8(const char *name);\n> int same_encoding(const char *, const char *);\n> diff --git a/wt-status.c b/wt-status.c\n> index 58461e0..672f83b 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -325,13 +325,13 @@ static const char *wt_status_diff_status_string(int status)\n> \t}\n> }\n> \n> -static int maxwidth(const char *(*label)(int), int minval, int maxval)\n> +static size_t maxwidth(const char *(*label)(int), int minval, int maxval)\n> {\n> \tint result = 0, i;\n> \n> \tfor (i = minval; i <= maxval; i++) {\n> \t\tconst char *s = label(i);\n> -\t\tint len = s ? utf8_strwidth(s) : 0;\n> +\t\tsize_t len = s ? utf8_strwidth(s) : 0;\n> \t\tif (len > result)\n> \t\t\tresult = len;\n> \t}\n> @@ -345,7 +345,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n> \tstruct wt_status_change_data *d = it->util;\n> \tstruct strbuf onebuf = STRBUF_INIT;\n> \tstatic char *padding;\n> -\tstatic int label_width;\n> +\tstatic size_t label_width;\n> \tconst char *one, *how;\n> \tint len;\n> \n> @@ -360,7 +360,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n> \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n> \n> \thow = wt_status_unmerged_status_string(d->stagemask);\n> -\tlen = label_width - utf8_strwidth(how);\n> +\tlen = label_width - cast_size_t_to_int(utf8_strwidth(how));\n> \tstatus_printf_more(s, c, \"%s%.*s%s\\n\", how, len, padding, one);\n> \tstrbuf_release(&onebuf);\n> }\n> @@ -429,7 +429,7 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n> \twhat = wt_status_diff_status_string(status);\n> \tif (!what)\n> \t\tBUG(\"unhandled diff status %c\", status);\n> -\tlen = label_width - utf8_strwidth(what);\n> +\tlen = label_width - cast_size_t_to_int(utf8_strwidth(what));\n> \tassert(len >= 0);\n> \tif (one_name != two_name)\n> \t\tstatus_printf_more(s, c, \"%s%.*s%s -> %s\",\n> \n> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca\n\n"},{"id":"549087","messageId":"xmqq4ihkpjn3.fsf@gitster.g","threadId":"66068","inReplyTo":"e971400e-6d23-463f-ae9c-a21d3c5a3563@gmail.com","subject":"Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-27T14:55:12Z","receivedAt":"2026-07-27T14:55:15Z","isPatch":true,"body":"Phillip Wood writes:\n\n> I think it would be much better to remove the TODO comment as Junio \n> previously suggested and instead add some documentation to the function \n> explaining (a) why it is appropriate for it to return an int; (b) why we \n> must use the cast_size_t_to_int() helper to prevent overflows (see the \n> commit that added that comment).\n\nThanks, especially for (b) above. That needs to be stressed if we\nare to go in that direction.\n"},{"id":"549092","messageId":"DK9HJ1A58HMD.2CDVOK50X2UMH@gmail.com","threadId":"66068","inReplyTo":"e971400e-6d23-463f-ae9c-a21d3c5a3563@gmail.com","subject":"Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T16:13:28Z","receivedAt":"2026-07-27T16:13:35Z","isPatch":true,"body":"On Mon Jul 27, 2026 at 6:21 PM IST, Phillip Wood wrote:\n\n>> diff --git a/builtin/blame.c b/builtin/blame.c\n>> index 48d5251..83e4dd6 100644\n>> --- a/builtin/blame.c\n>> +++ b/builtin/blame.c\n>> @@ -564,7 +564,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,\n>> \t\t\t\t\tname = ci.author_mail.buf;\n>> \t\t\t\telse\n>> \t\t\t\t\tname = ci.author.buf;\n>> -\t\t\t\tpad = longest_author - utf8_strwidth(name);\n>> +\t\t\t\tpad = longest_author - cast_size_t_to_int(utf8_strwidth(name));\n>> \t\t\t\tprintf(\" (%s%*s %10s\",\n>> \t\t\t\t name, pad, \"\",\n>> \t\t\t\t format_time(ci.author_time,\n>\n> To me this example perfectly illustrates why changing the return value \n> of utf8_strwidth() is a bad idea. The return value is pretty much always \n> used to calculate a padding to pass to printf() which expects an int. By \n> changing the return value you're forcing all the callers to do the \n> conversion themselves which is a bug waiting to happen. I'm also far \n> from convinced that the conversions in this patch are complete: grepping \n> for 'utf8_strn\\{0,1\\}width' turns up several calls which do not appear \n> to be correctly converted here. For example:\n>\n> builtin/worktree.c: display[i].width = utf8_strwidth(buf.buf);\n>\n> where \"width\" is an int.\n\nI had intentionally left out some sites which did not seem could have\nany impact by implicit conversions as there are other examples of such\ncases where the return value of `strlen` is being assigned to an int\nvariable. Example:\n\nin combine-diff.c (where len is an int):\n\tif (len < 0)\n\t\tlen = strlen(line);\n\nin builtin/update-index.c:\n\tint namelen = strlen(path);\n\nand other such examples.\n>\n> I think it would be much better to remove the TODO comment as Junio \n> previously suggested and instead add some documentation to the function \n> explaining (a) why it is appropriate for it to return an int; (b) why we \n> must use the cast_size_t_to_int() helper to prevent overflows (see the \n> commit that added that comment).\nThis can result in issues down the line and I had mentioned so in a\nprevious mail but wanted to try it with v3 since I had already been\nworking on it. I'll send a new patch to remove the TODO. This change\nmight just not be worth after all.\n\nThanks,\nHardik\n"},{"id":"549093","messageId":"DK9HO7JD6QT3.1ATJX1OLR8YBJ@gmail.com","threadId":"66068","inReplyTo":"xmqq4ihkpjn3.fsf@gitster.g","subject":"Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T16:20:14Z","receivedAt":"2026-07-27T16:20:22Z","isPatch":true,"body":"On Mon Jul 27, 2026 at 8:25 PM IST, Junio C Hamano wrote:\n> Phillip Wood writes:\n>\n>> I think it would be much better to remove the TODO comment as Junio \n>> previously suggested and instead add some documentation to the function \n>> explaining (a) why it is appropriate for it to return an int; (b) why we \n>> must use the cast_size_t_to_int() helper to prevent overflows (see the \n>> commit that added that comment).\n>\n> Thanks, especially for (b) above. That needs to be stressed if we\n> are to go in that direction.\n\nShould this be documented in a new adoc file in the technical\ndocumentation directory?\n\nThanks,\nHardik\n"},{"id":"549099","messageId":"xmqqtspkgqgp.fsf@gitster.g","threadId":"66068","inReplyTo":"DK9HO7JD6QT3.1ATJX1OLR8YBJ@gmail.com","subject":"Re: [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-27T19:52:38Z","receivedAt":"2026-07-27T19:52:41Z","isPatch":true,"body":"\"Hardik Kumar\" writes:\n\n> On Mon Jul 27, 2026 at 8:25 PM IST, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>>\n>>> I think it would be much better to remove the TODO comment as Junio \n>>> previously suggested and instead add some documentation to the function \n>>> explaining (a) why it is appropriate for it to return an int; (b) why we \n>>> must use the cast_size_t_to_int() helper to prevent overflows (see the \n>>> commit that added that comment).\n>>\n>> Thanks, especially for (b) above. That needs to be stressed if we\n>> are to go in that direction.\n>\n> Should this be documented in a new adoc file in the technical\n> documentation directory?\n\nThe best thing for the new comment to do is to replace the misguided\nTODO comment that led us to this exercise.\n\n"},{"id":"549102","messageId":"20260727211520.84289-1-hardikxk@gmail.com","threadId":"66068","inReplyTo":"20260726123427.173877-1-hardikxk@gmail.com","subject":"[PATCH v4] utf8: replace utf8_strwidth todo with descriptive comment","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-27T21:15:20Z","receivedAt":"2026-07-27T21:15:51Z","isPatch":true,"body":"The `utf8_strwidth()` function is used in multiple places that all\nexpect the function to return an int. The result is directly used for\npadding and width calculations and passed to `printf()` calls. All\nthese operations expect the function to return an int value. Changing\nthe return type here requires changing the types of all the callers and\nother additional variables, that depend on the results from this\nfunction directly or indirectly, to avoid overflow by implicit\nconversions.\n\nThe comment precisely explains the reason why the explicit conversion is\ndone.\n\n- Remove an old TODO that is no longer feasible.\n- Add a comment explaining the behaviour and reason of the allowed\nexpression.\n\nSigned-off-by: Hardik Kumar \n---\nchanges in v4:\n- drop the todo implementation and remove from codebase.\n- replace the todo with a reasonable explanation for the current\napproach and why its not worth the change.\n\n utf8.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex 96460cc..1b55bd4 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -227,8 +227,9 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n \t}\n \n \t/*\n-\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n-\t * return `size_t` instead of `int`.\n+\t * The function is used in multiple locations where the callers\n+\t * expect the result to be a signed int value. We cast the\n+\t * result to an int to avoid changing signatures of all callers.\n \t */\n \treturn cast_size_t_to_int(string ? width : len);\n }\n-- \n2.55.0\n\n"},{"id":"549139","messageId":"c8fb2eba-c1c8-4f59-b467-e6d4766623d8@gmail.com","threadId":"66068","inReplyTo":"20260727211520.84289-1-hardikxk@gmail.com","subject":"Re: [PATCH v4] utf8: replace utf8_strwidth todo with descriptive comment","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-07-28T15:41:11Z","receivedAt":"2026-07-28T15:41:15Z","isPatch":true,"body":"Hi Hardik\n\nOn 27/07/2026 22:15, Hardik Kumar wrote:\n> The `utf8_strwidth()` function is used in multiple places that all\n> expect the function to return an int. The result is directly used for\n> padding and width calculations and passed to `printf()` calls. All\n> these operations expect the function to return an int value. Changing\n> the return type here requires changing the types of all the callers and\n\ns/requires/would require/\n\n> other additional variables, that depend on the results from this\n> function directly or indirectly, to avoid overflow by implicit\n> conversions.\n> \n> The comment precisely explains the reason why the explicit conversion is\n> done.\n\nI don't think this comment, or the lines below add anything useful to \nthe message. It would be better to say something like\n\nAs we do not want to change the return type, update the comment to \nexplain that and the need for the explicit cast.\n\n> - Remove an old TODO that is no longer feasible.\n> - Add a comment explaining the behaviour and reason of the allowed\n> expression.\n> \n> Signed-off-by: Hardik Kumar \n> ---\n> changes in v4:\n> - drop the todo implementation and remove from codebase.\n> - replace the todo with a reasonable explanation for the current\n> approach and why its not worth the change.\n> \n> utf8.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/utf8.c b/utf8.c\n> index 96460cc..1b55bd4 100644\n> --- a/utf8.c\n> +++ b/utf8.c\n> @@ -227,8 +227,9 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n> \t}\n> \n> \t/*\n> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n> -\t * return `size_t` instead of `int`.\n> +\t * The function is used in multiple locations where the callers\n> +\t * expect the result to be a signed int value. We cast the\n> +\t * result to an int to avoid changing signatures of all callers.\n\nThe last sentence does not really capture the reasons given in the \nmessage of the commit that added this comment. If you haven't done so \nalready you should read it - see 937b71cc8b (utf8: fix overflow when \nreturning string width, 2022-12-01). The fundamental reason to call \ncast_size_t_to_int(), rather than relying on an implicit conversion to \nthe return type, is not about changing signatures, it is about avoiding \nan overflow that caused git to crash.\n\nWhen you send a new version of the patch please CC everyone who \ncommented on previous versions so they don't have to trawl the list to \nfind it.\n\nThanks\n\nPhillip\n\n\n\n> \t */\n> \treturn cast_size_t_to_int(string ? width : len);\n> }\n\n"},{"id":"549153","messageId":"20260728170228.31410-1-hardikxk@gmail.com","threadId":"66068","inReplyTo":"20260726123427.173877-1-hardikxk@gmail.com","subject":"[PATCH v5] utf8: replace utf8_strwidth todo with descriptive comment","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-28T17:02:28Z","receivedAt":"2026-07-28T17:02:52Z","isPatch":true,"body":"The `utf8_strwidth()` function is used in multiple places that all\nexpect the function to return an int. The result is directly used for\npadding and width calculations and passed to `printf()` calls. All\nthese operations expect the function to return an int value.\n\nAs we do not want to change the return type of function and its callers,\nadd a comment to explain that the need for an explicit cast is to avoid\ninteger overflow that caused git to crash.\n\n- drop the todo implementation and remove from codebase.\n- replace the todo with a reasonable explanation for the current\napproach and why its not worth the change.\n\nSigned-off-by: Hardik Kumar \n---\nchanges in v5:\n- update the comment to better explain the reason for explicit casting.\n- improve commit message with a reasonable explanation of the patch.\n\n utf8.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/utf8.c b/utf8.c\nindex 96460cc..d82e54d 100644\n--- a/utf8.c\n+++ b/utf8.c\n@@ -227,8 +227,9 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n \t}\n \n \t/*\n-\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n-\t * return `size_t` instead of `int`.\n+\t * The function is used in multiple locations where the callers\n+\t * expect the result to be a signed int value. We explicitly\n+\t * cast the the result to avoid integer overflow.\n \t */\n \treturn cast_size_t_to_int(string ? width : len);\n }\n-- \n2.55.0\n\n"},{"id":"549154","messageId":"DKAD8F8VLOMQ.3KKRGKVG6UT02@gmail.com","threadId":"66068","inReplyTo":"c8fb2eba-c1c8-4f59-b467-e6d4766623d8@gmail.com","subject":"Re: [PATCH v4] utf8: replace utf8_strwidth todo with descriptive comment","fromName":"Hardik Kumar","fromEmail":"hardikxk@gmail.com","sentAt":"2026-07-28T17:04:12Z","receivedAt":"2026-07-28T17:04:19Z","isPatch":true,"body":"On Tue Jul 28, 2026 at 9:11 PM IST, Phillip Wood wrote:\n\n> The last sentence does not really capture the reasons given in the \n> message of the commit that added this comment. If you haven't done so \n> already you should read it - see 937b71cc8b (utf8: fix overflow when \n> returning string width, 2022-12-01). The fundamental reason to call \n> cast_size_t_to_int(), rather than relying on an implicit conversion to \n> the return type, is not about changing signatures, it is about avoiding \n> an overflow that caused git to crash.\n\nI did check that commit before. My attempt at explaining the reason\nwasn't quite right. I have improved it in the next patch.\n\nThanks for the review.\nHardik\n"},{"id":"549160","messageId":"xmqq33x3dlbw.fsf@gitster.g","threadId":"66068","inReplyTo":"c8fb2eba-c1c8-4f59-b467-e6d4766623d8@gmail.com","subject":"Re: [PATCH v4] utf8: replace utf8_strwidth todo with descriptive comment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-28T18:24:03Z","receivedAt":"2026-07-28T18:24:05Z","isPatch":true,"body":"Phillip Wood writes:\n\n> I don't think this comment, or the lines below add anything useful to \n> the message. It would be better to say something like\n>\n> As we do not want to change the return type, update the comment to \n> explain that and the need for the explicit cast.\n\nPerfect ;-)\n\n>> diff --git a/utf8.c b/utf8.c\n>> index 96460cc..1b55bd4 100644\n>> --- a/utf8.c\n>> +++ b/utf8.c\n>> @@ -227,8 +227,9 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)\n>> \t}\n>> \n>> \t/*\n>> -\t * TODO: fix the interface of this function and `utf8_strwidth()` to\n>> -\t * return `size_t` instead of `int`.\n>> +\t * The function is used in multiple locations where the callers\n>> +\t * expect the result to be a signed int value. We cast the\n>> +\t * result to an int to avoid changing signatures of all callers.\n>\n> The last sentence does not really capture the reasons given in the \n> message of the commit that added this comment. If you haven't done so \n> already you should read it - see 937b71cc8b (utf8: fix overflow when \n> returning string width, 2022-12-01). The fundamental reason to call \n> cast_size_t_to_int(), rather than relying on an implicit conversion to \n> the return type, is not about changing signatures, it is about avoiding \n> an overflow that caused git to crash.\n\nThe comment should also answer why the callers want an int, and\nwhether that is a legitimate need. Topics the comment may want to\ncover include:\n\n - Callers want display width; we will never deal with output\n wider than 2 billion columns, so int is adequate, provided we\n do not cause bugs due to integer wraparound.\n\n - The return value is used to compute width in constructs like:\n\n printf(\"%*s\", width, string)\n\n which requires int, not size_t. Instead of forcing these\n callers to call cast_size_t_to_int() individually, this\n function should return int after ensuring the value is correct\n without wraparound.\n\nThis is in addition to explaining why we want cast_size_t_to_int(),\nas you described above.\n\n> When you send a new version of the patch please CC everyone who \n> commented on previous versions so they don't have to trawl the list to \n> find it.\n\nThanks.\n"}]}