public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Jeff Davis <[email protected]>
Cc: [email protected]
Subject: Re: Small patch to improve safety of utf8_to_unicode().
Date: Wed, 8 Jul 2026 09:48:02 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>



> On Jul 7, 2026, at 17:56, Jeff Davis <[email protected]> wrote:
> 
> On Fri, 2026-06-26 at 12:38 +0800, Chao Li wrote:
>> Sounds like 0001 will be back patched. In that case, the commit
>> message "defend against invalid UTF8” seems too broad. Does it make
>> sense to add some brief description about the defend behavior to the
>> function header comment and the commit message?
> 
> Committed 0001 and backported to 17. I think adding too much
> explanation about behavior we don't expect to actually see would just
> add confusion.
> 
>> Then I continue to review 0002-0005:
>> 
>> 0002 - overall looks good. A small comment is:
>> ```
>> + for (int i = offset; i > 0;)
>> 
>> + for (int i = offset + ulen; i < len;)
>> ```
> 
> Committed 0002 and backported to 18. The 'int' is pre-existing, so I
> left it as-is.
> 
>> 0003 - looks good.
> 
> Committed to master only.
> 
>> 0004 - looks good. This commit introduces a new helper utf8decode()
>> that will resolve my previous concern on 0001.
>> 
>> 0005 - Mostly looks good. This commit applies the new help and my
>> previous concern is resolved. But from what you talked, I guess 0004
>> and 0005 will only be pushed to HEAD.
>> 
>> Just one tiny comment on 0005:
>> ```
>> + /* invalid UTF8: surrogates */
>> + needed = unicode_strfold(NULL, 0, "abc\xED\xA0\x81xyz", 7,
>> &consumed, false);
>> + Assert(needed == 3 && consumed == 3);
>> ```
>> 
>> This test passes a 10-char string but uses 7 as srclen. I know that
>> doesn’t affect the test result, but it just adds unnecessary
>> confusion to readers. So maybe change 7 to 10 to reflect to the real
>> string length.
> 
> Thank you, attached with fix.
> 
> I'd like to wait for more comments before I commit these last two
> patches, to see if the functions are generally useful for other callers
> as well.
> 
> Regards,
> Jeff Davis
> 
> <v6-0001-Validating-iterator-friendly-UTF8-encoder-decoder.patch><v6-0002-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch>

V6 looks good to me.

Best regards,
—
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

view thread (18+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Small patch to improve safety of utf8_to_unicode().
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox