From: Junio C Hamano <gitster@pobox.com>
To: Hardik Kumar <hardikxk@gmail.com>
Cc: git@vger.kernel.org, l.s.r@web.de, pabloosabaterr@gmail.com
Subject: Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.
Date: Sun, 26 Jul 2026 21:02:23 -0700 [thread overview]
Message-ID: <xmqqbjbtqdv4.fsf@gitster.g> (raw)
In-Reply-To: <xmqqpl09s3cc.fsf@gitster.g> (Junio C. Hamano's message of "Sun, 26 Jul 2026 17:06:43 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> The goal looks attractive on the surface, and the change to make
> utf8_strwidth() and utf8_strnwidth() return 'size_t' clears an
> existing TODO. However, the updates to the call sites to support
> this change introduce several bugs due to unsigned integer underflow
> and incorrect mixed-sign comparisons.
Having said that, we need to remember that these two functions are
not designed for anything more than what fits on a single line. The
only reason they exist in our codebase is because their callers want
to measure the display width of a string, so that they can align
elements on a line vertically with the corresponding elements on the
previous and next lines.
This does not mean we do not need to support more than 80 columns
;-), but they surely do not have to support a 2-billion-column-wide
display.
Quite honestly, I have to say that this topic has a very low
expected benefit in practice, while it costs us quite a lot by
having to carefully code and even more carefully review. If we have
to endure so many new bugs in the callers just to clear an existing
TODO, we might be better off not doing so and relying on the "safe
cast from size_t down to int that barfs if the quantity does not fit
in an int" protection.
next prev parent reply other threads:[~2026-07-27 4:02 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
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
2026-07-26 15:52 ` Hardik Kumar
2026-07-26 19:57 ` [PATCH v2] utf8: use size_t for string width methods and callee sites Hardik Kumar
2026-07-27 0:06 ` Junio C Hamano
2026-07-27 4:02 ` Junio C Hamano [this message]
2026-07-27 6:42 ` Hardik Kumar
2026-07-27 1:06 ` Pablo Sabater
2026-07-27 6:40 ` Hardik Kumar
2026-07-27 6:59 ` [PATCH v3] utf8: make utf8_strwidth() and utf8_strnwidth() return size_t Hardik Kumar
2026-07-27 7:04 ` Hardik Kumar
2026-07-27 12:51 ` Phillip Wood
2026-07-27 14:55 ` Junio C Hamano
2026-07-27 16:20 ` Hardik Kumar
2026-07-27 16:13 ` Hardik Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqbjbtqdv4.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=hardikxk@gmail.com \
--cc=l.s.r@web.de \
--cc=pabloosabaterr@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.