Re: multibyte support (round 3)

[Assaf Gordon]

Hello Pádraig,

Thanks for the review!

> On Sep 19, 2016, at 09:25, Pádraig Brady <address@hidden> wrote:
> 
> A general point is I'm still of the opinion that
> it would be better to have all conversion and checking
> in unorm(1), thus simplifying/optimizing the checking/processing
> in all other utils.  It would be good to get an idea
> of performance/overhead as the patches progress.

In the latest code, the multibyte parsing in 'mbbuffer' does no conversion
(except UTF-16 surrogate in Cygwin). It calls mbrtowc(3)
and returns the result.
Error checking is done, because it was my understanding that the coreutils
program should never reject invalid sequences - instead they must be processed
as single octets and output as-is.

The added complexity in 'mbbuffer' is mainly reading from the file
and ensuring there are always enough input octets for mbrtowc
(this is very similar to the implementation in the current i18n patch).

I will try to compile some performance measurements soon.

> I'm thinking this could be merged in the next major
> version of coreutils, which would come after the
> next minor release which hopefully will be released
> in the next few weeks.

Thank you - obviously there is no rush, and this will need a lot more testing.


> A couple of points on very quick review:
> 
> is_utf8_locale_name()
>  In gnulib, "UTF-8" is commented as the only
>  variant that needs to be checked from the
>  return of locale_charset()
> 
> is_valid_mb_character()
>  There can invalid characters in many single byte locales:
>  For example 81,8d,90,9d,93 in cp1252, as shown by:
>  recode -lh cp1252 | grep -C1 -F "     "

> 
> mbtowc_utf16()
>  The assert() triggers -Werror=suggest-attribute=noreturn
>  when not on cygwin, so it's better to avoid compiling
>  that function altogether on other platforms.  I.E. put
>  #ifdef HAVE_UTF16_SURROGATES around the function definition
>  (and declaration to get a compile error rather than a link error)
> 

All good points, I'll fix them.

regards,
 - assaf