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

From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: Jul 26, 2026, 14:52 UTC
Message-ID: <DK8L6JM14UNS.16B15DIOFW1K5@gmail.com>
In-Reply-To: <20260726123427.173877-1-hardikxk@gmail.com>

On Sun Jul 26, 2026 at 2:34 PM CEST, Hardik Kumar wrote:

> The patch changes the return types of `utf8_strwidth()` and

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

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

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

Change their signatures to return size_t instead.

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

Show 33 quoted lines

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

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

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

Show 13 quoted lines

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

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

We can check who their callers are with:

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

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

(there are more)

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

Show 20 quoted lines

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

The signature change looks ok.

Regards, Pablo