On 11/20/18 5:01 PM, Eric Blake wrote:
> [adding Markus in CC, since git didn't do it automatically from the
> 'Reported-by']
>
> On 11/20/18 3:28 PM, John Snow wrote:
>>
>>
>> On 11/20/18 3:36 PM, Eric Blake wrote:
>>> While most developers are now using UTF-8 environments, it's
>>> harder to guarantee that error messages will be output to
>>> a multibyte locale. Rather than risking error messages that
>>> get corrupted into mojibake when the user runs qemu in a
>>> non-multibyte locale, let's stick to straight ASCII error
>>> messages, rather than assuming that our use of UTF-8 in source
>>> code string constants will work unchanged in other locales.
>>>
>>> Reported-by: Markus Armbruster <arm...@redhat.com>
>>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>>> ---
>>> hw/misc/tmp105.c | 2 +-
>>> hw/misc/tmp421.c | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>
>>
>> Do we have any policy in place to prohibit this in the future?
>> (Presumably a policy that is automatic and won't interfere with QEMU
>> localization efforts which may rightly attempt to use UTF-8 for those
>> locales.)
>
> Not that I know of.
>
>>
>> Do you have a script or trick to find utf-8 containing strings in our
>> source?
>
> Markus found these two, probably by reading over a list resulting from
> his claim of finding 217 out of 6455 files (53 of them binary, which
> don't count):
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04017.html
>
> My quick and dirty attempt, which does not quite reproduce his numbers:
>
> $ LC_ALL=C git grep -l $'[\x80-\xff]' | wc
> 279 279 7490
>
> Thus, by forcing a unibyte locale (where encoding errors are impossible)
> with sane range expressions (POSIX says only the C locale is required to
> interpret regex ranges according to byte value - all bets are off in
> other locales) and using $'' to type non-UTF-8 bytes into my search, I
> found 279 files with at least one byte outside of ASCII. But the use of
> -l has no easy way to filter which of those files are binary; while
> dropping -l claims 2138 "lines" with non-ASCII, which gets tedious to
> scroll through, especially considering there ARE binary files in the mix.
>
> Narrowing the search to a more specific pattern:
>
> $ LC_ALL=C git grep $'".*[\x80-\xff].*"' | grep -v 'Binary file' | wc
> 129 685 8808
>
> is a bit more manageable, with MOST of the hits in pc-bios/qemu.rsrc
> (false positive hits, due to interesting? comments), in po/ (which
> doesn't count), or in scripts/ for python. And the proof for THIS patch:
>
> $ LC_ALL=C git grep -l $'".*[\x80-\xff].*"' origin -- '**/*.[ch]' | cat
> origin:hw/misc/tmp105.c
> origin:hw/misc/tmp421.c
>
Aha!
Thanks. I was surprised we've managed to introduce only two cases of
this so far. Your patch seems complete in this case.
Since I took the time to read along, I may as well:
Reviewed-by: John Snow <js...@redhat.com>
>>
>> Only curious, don't hold this patch up on my account. I'm not raising a
>> challenge.
>
> Maybe checkpatch.pl could be taught to do a similar check?
>
