Looks good to me now. Thanks!

I wasn't planning to include a description in the commit body, but perhaps a brief explanation like the one above would be useful?

This is required to prevent function-like macros with the same name from being expanded in the definitions once they become active in a later C mode. Without the parentheses, the declarator is rewritten and will not compile. This style is used for similar cases such as mempcpy() too.

Diff 7/8 looks good on the INIT-REBOOT RELEASE/disassoc path, but ignore ipv6only doesn't seem to be a real opt-out.

Same here, I’ve been testing it for a while now, and it has my approval as well. Thanks a lot for all the effort, Pouria!

Drop sizeof(mbstate_t) - sizeof(_Char8State) >= 64 for now

Mark review comment on _Mbrtoc8Extra as done, as it is no longer applicable following the changes that were implemented.

Rename _C8rtombExtra to _Char8State for consistency

Mark several review notes as resolved: Björn Höhrmann’s DFA implementation, __cpp_char8_t rationale, u8'\0' clarification, c8rtomb licensing, data type improvements, the states enum, urtf8d table formatting, and unit test improvements.

Thanks for your notes! Sure, I’ll keep them in mind and take care of those herein in a minute.

Address the remaining review points

Address the three promised review points

Sorry if I made some modifications to what you sent me via email. I actually thought I was allowed to, as it isn't finalised yet and we're working through it together. I'm just trying to make sure we're not rushing things and that we're on the same page as we go along, for future maintenance and stuff we'd be working on together. I certainly don't mean to be rude or inattentive. Also, thank you for putting so much time and effort into reviewing everything, explaining things, and discussing it all with me, and being this much receptive and kind throughout. I really appreciate it.

Nice move. Thanks!

Default MK_ICONV=yes now runs mbrtoc8/c8rtomb through Citrus like c16/c32, and unsurprisingly, we now inherit its bugs as a result: null s resets, ill-formed input often comes back as (size_t)-2, 5-/6-byte UTF-8 (as with RFC 2279 §2—obsoleted by RFC 3629 §3) and values above U+10FFFF are accepted, U+FEFF fails, and each mbstate_t leaks. Those need an iconv fix, though, so not c8 special cases. FWIW, WITHOUT_ICONV c8rtomb stays C23 in (c8rtomb.c / mbrtoc8.c), and mbrtoc8 matches mbrtoc16.

Brilliant work. Thanks!

Thanks, Robert! Sure, I will do.

Move __WCHAR_WIDTH into sys/sys/_types.h

If the size is the same on all platforms, it should not be in the platform-specific _types.h files.

Excellent work on this. Thanks!

Looks good to me. Thanks!

In D59288#1360867, @fuz wrote:

Approved for commit following @kib's review. I did not review the code, so only Approved by from me please.

Add new symbols in alphabetical order and apply @kib's fenv.3 suggestion

Is it reasonable to provide the inline versions of these methods for complicated cases, like x86? Could it be always a function call?

Address @kib's inline comments

This should be reviewed by kib, andy (for ARM), jrtc27 (for riscv64), jhibbits (for ppc), and kargl (for msun).

Document mbrtoc8() and c8rtomb() functions

This seems to be correct at first glance, but I think it can be greatly simplified and improved in performance.. I did a lot of research on Unicode validation and transcoding and had originally meant to take a stab at this function, but then forgot about it.

I'll think about it some more and then let you know what I came up with.

I’ll take care of manpages in the coming days.

Sure!

Sure thing. I'll take care of it.

Thanks a lot for your notes, Devin. I can see it much more clearly now, and I understand why 1:1 is the preferred approach. I’ve just updated the summary with the message for the squashed commit, and I’ll push my changes shortly.

I do have some questions about the summary on this review though. Seeing 3 separate commit sections, how do you intend to commit this?

Looks good to me. Thanks!

Update commit messages

Address @fuz comments

• bsearch.3: Document bsearch_b extension
• libc: Add bsearch_s() from Annex K
• libc: Add tests for bsearch(), bsearch_b(), and bsearch_s()

Looks good to me, but while you're here, could we also map IFT_LOOP -> LINUX_ARPHRD_LOOPBACK so a Linux ip link show lo under the linuxulator does not look like FireWire? The constant already exists. Fine as a follow-up if you want to keep this patch to bridge/vlan/stf.

Looks good to me. Thanks!

It's brilliant. Cheers!.

Sure, thanks!

The bsearch_b documentation is handled in a separate commit. It displays correctly for me here in Phabricator, but I’ll shortly update the description with the proposed commit messages, as we’ve agreed to use that convention to make reviews easier and clearer. And as for the tests, sure, I’m working on it; I do my best to submit the commit by tonight.