I had this symbol as C23 only for a reason. I think it was because things go really wrong in C++ land if this header defines char8_t otherwise. Please check this carefully (e.g. by building world with the patch applied) or just leave things as they were.

It would be great if the design patterns and symbol naming could be similar to the existing files c16rtomb.c and c32rtomb.c for consistency.

That 64 is a magic number. Where is it from? Is there a macro we can use instead?

I don't think this comment is relevant. \0 is the string terminator, which does not end up being transcoded. The function is meant to be restartable, meaning, that you can deliver input in chunks that may not themselves all start or end at boundaries of full characters (if that wasn't the case, then the mbstate_t argument would be pointless, as you'd always be in initial state when calling this function).

What is the meaning of these structure members?

What we do now is the same idea as char16_t: _types.h sets _CHAR8_T_DECLARED when the language already has the type, and uchar.h only typedefs if that flag is clear. We key it off __cpp_char8_t, not __cplusplus >= 202002L, so -fchar8_t / -fno-char8_t are correct. Prototypes are emitted in C++ only when __cpp_char8_t is set, so they always use the builtin, never unsigned char pretending to be char8_t.

C++11/17 without the builtin do get typedef unsigned char char8_t if they include <uchar.h>. That cannot collide with a keyword (there isn’t one). It can collide with a user identifier named char8_t, same class of issue as char16_t before C++11. The keyword case is protected.

So, in fact, by improving it slightly and guarding against __cpp_char8_t, I tried fixing a rare bug that had been lurking there for a while. I ran pretty much every test that sprang to my mind, and I think this is considerably safer than the previous check. Bu I would really appreciate it if you could take a look as well and let me know if you spot anything I’ve missed.

Sure, I can rename _C8rtombExtra / _Mbrtoc8Extra to _Char8State.

It’s half the size of the mbstate_t blob, and I think that’s an OK promise. Perhaps I could do this:

#define _MBSTATE_CONV_PREFIX (sizeof(mbstate_t) / 2)

I’m still not sure it’s the best we can do, though. I’d rather use sizeof(extra) + sizeof(_ConversionState) <= sizeof(mbstate_t), but that would pull in the iconv types, and our case here is MK_ICONV=no.

What would you do, Robert?

But u8'\0' is a char8_t value, not a C-string terminator. Also, restartability remains unchanged. If the comment is misleading, we can certainly drop it altogether (or perhaps just the second sentence). However, if I’m missing something here, I’d appreciate it if you could point it out and correct me.

nleft / next / buf are the leftover UTF-8 units after we already returned the first one ((size_t)-3), and incomp means mbrtoc32 last returned (size_t)-2, so a following NUL is EILSEQ instead of being swallowed by iconv.

Yeah ok I guess that makes sense. I mixed up strings and characters while reading this one; it was late.

Great! Please add comments documenting this meaning so others have an easier time following your code.

This needs to have an _iconv version for when we use the MK_ICONV codepath. This code is very strange.

You can inline this function and call the underlying primitive directly to save time.
We can assume we are in the non-iconv variant of libc if this code is built; you'll need to provide an _iconv variant for the case when iconv is enabled.

The state does not need to be a uint32_t. And unsigned integer type is fine and it's a good idea to have the platform pick a good one. I changed it to unsigned for this reason. Why did you undo the change?

This appears to be unrelated?

Right, we do not, but take a look at this section. Also, the MIT licence only requires the copyright notice and the permission notice (where for the latter we only use the SPDX short identifier).

Excerpt from the MIT licence:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

Besides, IMHO, going with a dual licence could be preferable, keeping both the standardised BSD-2-Clause for our new c8rtomb.c and the MIT licence for what we have imported. And for that I chose AND (although you had recommended OR) owing to the following excerpt from FreeBSD Licensing Policy:

OR should be used if the file has a choice of license and one license is selected. For example, some dtsi files are available under dual licenses:

// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause

AND should be used if the file has multiple licenses whose terms all apply to use the file. For example, if code has been incorporated by several projects, each with their own license:

// SPDX-License-Identifier: BSD-2-Clause AND MIT

So, to my understanding (and limited experience), URLs and/or any additional data provided, as long as they are not part of the copyright notice per se, seem to be fine to omit. I would really appreciate it if you could take another look and see if there is a chance I might be right here.

Well, I'm sorry, it was my oversight, I honestly missed your valid rationale for this. I just followed Höhrmann's use of uint32_t for no particular reason. I'll apply yours shortly. Thanks for explaining this, by the way.

Right. I picked these from Björn Höhrmann's post and thought they'd be descriptive enough without any comments or further explanation. Let me know if you'd like me to revert this change as well.

Yikes! It was just personal taste, looked cleaner to my eyes actually. Righto, I’ll revert it now :-))

Thanks. Leaving REJECT absorbing would make every subsequent call fail until the caller clears ps, whereas resetting to ACCEPT would let the next unit start a new sequence. So, since C23 §7.30.1.2 leaves the state after EILSEQ unspecified, I went with the latter, so that a single bad unit doesn’t poison the rest of the stream.

Righto, will do!

Nope. Quite the contrary, I consider it very important and related, and it has been here since my first diff. I told you about this here.

Sorry, I'm not going to budge on this one. Please put the “See also” back in. It's clearly part of the copyright notice. And even if it was not, it's really rude to remove this sort of stuff. If you look through files we adapted from other projects, you see that we generally keep the original copyright headers as they are, just adding our own stuff. We never remove existing copyright lines from files. Apart from often being a licensing requirement, this is the least we can do to respect the author's rights and wishes to be credited for their work.

In particular in German law, the author has the right to decide how he wishes to be credited (§ 13 UrhG). The author has clearly expressed his wish to be credited with these two lines and we should respect that.

As for the AND/OR thing, yours is right, I probably mixed it up due to sleep deprivation.

It's fine, don't worry about this. I went through the code before I sent it to you, reviewed it, and did some light copy-editing. So I was a bit surprising to see some of the changes I did reverted. In particular, I don't like how authors often use fixed-width types for no particular reason, so I changed the use to a platform-specific type to let the platform pick a good type.

Fixed-width types have a documentation signal. They say “this variable must have this width or it won't work.” If you use them when the variable does not in fact have to have this particular width, you're creating a “Chesterton's fence” during any sort of rework, wasting people's time. So I try to use fixed-width types only if they can be justified. Otherwise native types (int/long/...) or types like int32least_t may be more useful to document intent.

I mean it's probably fine without a comment, but the comment certainly help people get the intent in the first place. I am kind of puzzled why you went the extra mile to remove them.

As for the comment you put above it, I don't think it is super useful as it views the decoding from a less important angle. The most important bit is that the table parses UTF-8. It rejects all invalid UTF-8, so it's unclear why you mention only a few of the possible encoding errors you can do (as if it would not catch other issues, like stray or missing follow bytes?) The second thing is that the character class itself is used to strip off the UTF-8 tag bits, which makes this decoder much simpler than others. And now with the original comment restored, the rest of it is kind of redundant.

Yes indeed, good idea.

Right sorry, I forgot. I was late at night yesterday and I was tired.

Humble suggestion, certainly not legal advice: MIT and BSD-2-Clause are completely equivalent and there is absolutely zero to be gained from increasing the complexity of the legal code by dual licensing this. Already we are debating the difference between AND and OR and additional man hours will continue to be wasted on this issue in the future for no benefit in a "<permissive> {and | or} <permissive>" case. From FreeBSD policy side, it's not a special case for it to be MIT.

Personally, I really prefer BSD-2-Clause for new files in FreeBSD for simplicity and I also prefer leaving it at MIT if you're bringing in something that's MIT for the exact same reason. Again, just a suggestion.

OK, I’m convinced. It was just that the licence section in there, with no “See also”, made me think we could remove it. But alright, I’ll take care of it sharpish. Roger that.

Nicely put, agreed.

Right. I wanted to move the “See also” and thought I could perhaps bring it here, but, well, it didn’t work out nicely. Reverting it to your style.

And on the surprise this and the revert above caused, my apologies, Robert. I tend to revise a patch over and over while working through it, and occasionally make edits without a note explaining why. Not a comment on the edit itself, just an artefact of how I lean toward minimalism and iterative improvements, and not always for the better, admittedly. So please poke me if it ever happens and I'll explain myself.

Thinking about it more, I should've submitted the original version with just my updates, and flagged subjective changes like this as inline comments here instead so we could discuss. Next time ;-D

No worries at all, mate.

That makes sense. I’m convinced here as well. My preference for keeping the BSD-2-Clause, for consistency with the standardised and preferred licence, was really just a matter of taste, so largely subjective. OK, I’ll go with MIT, as initially done by Robert. Thanks a lot for your review, Alexander.

I’ll drop this entirely for now; we can add something more accurate later.