[COMMITTED] localedef: Add verbose messages for failure paths.

[Carlos O'Donell]

On 4/27/20 7:52 AM, Szabolcs Nagy wrote:
> The 04/26/2020 15:06, Carlos O'Donell via Libc-alpha wrote:
>> I'm pushing this patch as reviewed by Florian and Adhemerval.
>>
>> This is the last patch we had in gerrit.
>>
>> 8< --- 8< --- 8<
>> From 92954ffa5a5662fbfde14febd7e5dcc358c85470 Mon Sep 17 00:00:00 2001
>> From: Carlos O'Donell <carlos@redhat.com>
>> Date: Wed, 8 Jan 2020 13:24:42 -0500
>> Subject: [PATCH] localedef: Add verbose messages for failure paths.
>>
>> During testing of localedef running in a minimal container
>> there were several error cases which were hard to diagnose
>> since they appeared as strerror (errno) values printed by the
>> higher level functions.  This change adds three new verbose
>> messages for potential failure paths.  The new messages give
>> the user the opportunity to use -v and display additional
>> information about why localedef might be failing.  I found
>> these messages useful myself while writing a localedef
>> container test for --no-hard-links.
>>
>> Since the changes cleanup the code that handle codeset
>> normalization we add tst-localedef-path-norm which contains
>> many sub-tests to verify the correct expected normalization of
>> codeset strings both when installing to default paths (the
>> only time normalization is enabled) and installing to absolute
>> paths.  During the refactoring I created at least one
>> buffer-overflow which valgrind caught, but these tests did not
>> catch because the exec in the container had a very clean heap
>> with zero-initialized memory. However, between valgrind and
>> the tests the results are clean.
>>
>> The new tst-localedef-path-norm passes without regression on
>> x86_64.
>>
>> Change-Id: I28b9f680711ff00252a2cb15625b774cc58ecb9d
>> ---
>>  include/programs/xasprintf.h                  |  24 ++
>>  locale/Makefile                               |   3 +-
>>  locale/programs/localedef.c                   | 151 +++++------
>>  locale/programs/localedef.h                   |   1 +
>>  locale/programs/xasprintf.c                   |  34 +++
>>  locale/tst-localedef-path-norm.c              | 242 ++++++++++++++++++
>>  .../postclean.req                             |   0
>>  .../tst-localedef-path-norm.script            |   2 +
>>  support/Makefile                              |   3 +-
>>  support/support.h                             |   2 +
>>  support/support_paths.c                       |   7 +
>>  11 files changed, 394 insertions(+), 75 deletions(-)
>>  create mode 100644 include/programs/xasprintf.h
>>  create mode 100644 locale/programs/xasprintf.c
>>  create mode 100644 locale/tst-localedef-path-norm.c
>>  create mode 100644 locale/tst-localedef-path-norm.root/postclean.req
>>  create mode 100644 locale/tst-localedef-path-norm.root/tst-localedef-path-norm.script
> 
> locale/tst-localedef-path-norm consistently times out on the aarch64 buildbot.
> 
> is it ok to commit the attached fixup?

I'm OK with upgrading the test case timeout to 25s.

Florian does not agree with me, and would like to see if he can change
the charmap used to iso-88591-15 instead of UTF-8 (which is a large charmap)
and still get coverage of the same code.

The reason I used UTF-8 is because it tests the normalization code that
converts UTF-8 to utf8 (lowercase and drop dash). Which is also achieved by
iso-88591-15.

I think Florian can follow up with another patch to change the test case to
use iso-88591-15 and lower the timeout.

In the meantime I'm going to approve your fix to unblock your builders.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

-- 
Cheers,
Carlos.