* Re: [PATCH] change utf8_strwidth() return type to size_t
2026-07-26 12:34 [PATCH] change utf8_strwidth() return type to size_t Hardik Kumar
@ 2026-07-26 13:41 ` René Scharfe
2026-07-26 15:50 ` Hardik Kumar
2026-07-26 14:52 ` Pablo Sabater
1 sibling, 1 reply; 5+ messages in thread
From: René Scharfe @ 2026-07-26 13:41 UTC (permalink / raw)
To: Hardik Kumar, git
On 7/26/26 2:34 PM, Hardik Kumar wrote:
> The patch changes the return types of `utf8_strwidth()` and
> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions
> have been updated in the header file also.
>
> Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
> ---
> utf8.c | 13 ++++---------
> utf8.h | 4 ++--
> 2 files changed, 6 insertions(+), 11 deletions(-)
What about callers that still expect int? Are they all safe without
cast_size_t_to_int()?
>
> diff --git a/utf8.c b/utf8.c
> index 96460cc..1081573 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
> * string, assuming that the string is utf8. Returns strlen() instead
> * if the string does not look like a valid utf8 string.
> */
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> {
> const char *orig = string;
> size_t width = 0;
> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> if (glyph_width > 0)
> width += glyph_width;
> }
> -
> - /*
> - * TODO: fix the interface of this function and `utf8_strwidth()` to
> - * return `size_t` instead of `int`.
> - */
> - return cast_size_t_to_int(string ? width : len);
> + return (string) ? width : len;
Nit: Why the parentheses around "string"?
> }
>
> -int utf8_strwidth(const char *string)
> +size_t utf8_strwidth(const char *string)
> {
> return utf8_strnwidth(string, strlen(string), 0);
> }
> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
> const char *s)
> {
> size_t slen = strlen(s);
> - int display_len = utf8_strnwidth(s, slen, 0);
> + size_t display_len = utf8_strnwidth(s, slen, 0);
> int utf8_compensation = slen - display_len;
>
> if (display_len >= width) {
> diff --git a/utf8.h b/utf8.h
> index cf8ecb0..531e968 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */
>
> size_t display_mode_esc_sequence_len(const char *s);
> int utf8_width(const char **start, size_t *remainder_p);
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> -int utf8_strwidth(const char *string);
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> +size_t utf8_strwidth(const char *string);
> int is_utf8(const char *text);
> int is_encoding_utf8(const char *name);
> int same_encoding(const char *, const char *);
>
> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] change utf8_strwidth() return type to size_t
2026-07-26 12:34 [PATCH] change utf8_strwidth() return type to size_t Hardik Kumar
2026-07-26 13:41 ` René Scharfe
@ 2026-07-26 14:52 ` Pablo Sabater
2026-07-26 15:52 ` Hardik Kumar
1 sibling, 1 reply; 5+ messages in thread
From: Pablo Sabater @ 2026-07-26 14:52 UTC (permalink / raw)
To: Hardik Kumar, git
On Sun Jul 26, 2026 at 2:34 PM CEST, Hardik Kumar wrote:
> The patch changes the return types of `utf8_strwidth()` and
Regarding the presentation: "The patch changes...", try to avoid this
pattern, I think something like this would fit better:
utf8_strwidth() and utf8_strnwidth() return int, even though the value
they return is always non-negative:
- utf8_strnwidth() accumulates the width into a size_t and otherwise
returns its size_t len parameter,
- utf8_strwidth() just forwards its result.
Change their signatures to return size_t instead.
If you want to mention the TODO, I would add it after the '---'.
> `utf8_strnwidth()` to `size_t` (implementing a //TODO). Both functions
> have been updated in the header file also.
>
> Signed-off-by: Hardik Kumar <hardikxk@gmail.com>
> ---
> utf8.c | 13 ++++---------
> utf8.h | 4 ++--
> 2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/utf8.c b/utf8.c
> index 96460cc..1081573 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -208,7 +208,7 @@ int utf8_width(const char **start, size_t *remainder_p)
> * string, assuming that the string is utf8. Returns strlen() instead
> * if the string does not look like a valid utf8 string.
> */
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> {
> const char *orig = string;
> size_t width = 0;
> @@ -225,15 +225,10 @@ int utf8_strnwidth(const char *string, size_t len, int skip_ansi)
> if (glyph_width > 0)
> width += glyph_width;
> }
> -
> - /*
> - * TODO: fix the interface of this function and `utf8_strwidth()` to
> - * return `size_t` instead of `int`.
> - */
> - return cast_size_t_to_int(string ? width : len);
> + return (string) ? width : len;
nit: parentheses at "(string)" are unnecessary.
Also, cast_size_t_to_int() had an overflow check, we need to be sure
that no caller relies on that check. If you have checked for that,
please mention it in the commit message.
> }
>
> -int utf8_strwidth(const char *string)
> +size_t utf8_strwidth(const char *string)
> {
> return utf8_strnwidth(string, strlen(string), 0);
> }
> @@ -821,7 +816,7 @@ void strbuf_utf8_align(struct strbuf *buf, align_type position, unsigned int wid
> const char *s)
> {
> size_t slen = strlen(s);
> - int display_len = utf8_strnwidth(s, slen, 0);
> + size_t display_len = utf8_strnwidth(s, slen, 0);
We are fixing a caller here and that is correct.
But these functions that we've changed in this patch are called
throughout the codebase, we should fix those callers too.
We can check who their callers are with:
git grep -n -E 'utf8_str.?width'
builtin/repo.c:390: int value_width = utf8_strwidth(entry->value);
builtin/repo.c:395: int unit_width = utf8_strwidth(entry->unit);
builtin/repo.c:585: int title_name_width = utf8_strwidth(name_col_title);
builtin/repo.c:586: int title_value_width = utf8_strwidth(value_col_title);
(there are more)
From what I reviewed, no caller will break because of this, but I think
we should fix it for consistency.
> int utf8_compensation = slen - display_len;
>
> if (display_len >= width) {
> diff --git a/utf8.h b/utf8.h
> index cf8ecb0..531e968 100644
> --- a/utf8.h
> +++ b/utf8.h
> @@ -7,8 +7,8 @@ typedef unsigned int ucs_char_t; /* assuming 32bit int */
>
> size_t display_mode_esc_sequence_len(const char *s);
> int utf8_width(const char **start, size_t *remainder_p);
> -int utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> -int utf8_strwidth(const char *string);
> +size_t utf8_strnwidth(const char *string, size_t len, int skip_ansi);
> +size_t utf8_strwidth(const char *string);
> int is_utf8(const char *text);
> int is_encoding_utf8(const char *name);
> int same_encoding(const char *, const char *);
>
> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
The signature change looks ok.
Regards,
Pablo
^ permalink raw reply [flat|nested] 5+ messages in thread