Re: [PATCH v2] utf8: use size_t for string width methods and callee sites.

Junio C Hamano <[email protected]> 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.