locale: Fix localedef exit code (Bug 22292)

The error and warning handling in localedef, locale, and iconv
is a bit of a mess.

We use ugly constructs like this:
      WITH_CUR_LOCALE (error (1, errno, gettext ("\
cannot read character map directory `%s'"), directory));

to issue errors, and read error_message_count directly from the
error API to detect errors. The problem with that is that the
code also uses error to print warnings, and informative messages.
All of this leads to problems where just having warnings will
produce an exit status as-if errors had been seen.

To fix this situation I have adopted the following high-level
changes:
* All errors are counted distinctly.
* All warnings are counted distinctly.
* All informative messages are not counted.
* Increasing verbosity cannot generate *more* errors, and
  it previously did for errors conditional on verbose,
  this is now fixed.
* Increasing verbosity *can* generate *more* warnings.
* Making the output quiet cannot generate *fewer* errors,
  and it previously did for errors conditional on be_quiet,
  this is now fixed.
* Each of error, warning, and informative message has it's
  own function to call defined in record-status.h, and they
  are: record_error, record_warning, and record_verbose.
* The record_error function always records an error, but
  conditional on be_quiet may not print it.
* The record_warning function always records a warning,
  but conditional on be_quiet may not print it.
* The record_verbose function only prints the verbose
  message if verbose is true and be_quiet is false.

This has allowed the following fix:
* Previously any warnings were being treated as errors
  because they incremented error_message_count, but now
  we properly return an exit status of 1 if there are
  warnings but output was generated.

All of this allows localedef to correctly decide if errors,
or warnings were present, and produce the correct exit code.

The locale and iconv programs now also use record-status.h
and we have removed the WITH_CUR_LOCALE hack, and instead
have internal push_locale/pop_locale functions centralized
in the record routines.

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

26 files changed, 767 insertions, 489 deletions

diff --git a/ChangeLog b/ChangeLog
index d0bc31749e..fd3fe787df 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,61 @@
+2017-10-13  Carlos O'Donell  <carlos@redhat.com>
+
+	[BZ #22292]
+	* locale/programs/record-status.h: New file
+	* locale/programs/locale.c: Add comment.
+	* locale/programs/charmap-dir.c: Don't include error.h.
+	(charmap_opendir): Use record_error.
+	* locale/programs/charmap.c: Don't include error.h.
+	(charmap_read): Use record_error, and record_warning.
+	(parse_charmap): Likewise.
+	* locale/programs/ld-address.c: Don't include error.h.
+	(address_finish): Use record_error, and record_warning.
+	* locale/programs/ld-collate.c: Don't include error.h.
+	(collate_finish): Use record_error, and record_error_at_line.
+	* locale/programs/ld-ctype.c (ctype_finish): Use record_error.
+	(ctype_class_new): Likewise.
+	(ctype_map_new): Likewise.
+	(set_one_default): Likewise.
+	(set_class_defaults): Likewise.
+	(translit_flatten): Likewise.
+	(allocate_arrays): Use record_error, and record_verbose.
+	* locale/programs/ld-identification.c: Don't include error.h.
+	(indentation_finish): Use record_error and record_warning.
+	* locale/programs/ld-measurement.c: Don't include error.h.
+	(measurement_finish): Use record_error.
+	* locale/programs/ld-messages.c
+	(message_finish): Likewise.
+	* locale/programs/ld-monetary.c
+	(monetary_finish): Likewise.
+	* locale/programs/ld-name.c (name_finish): Use record_error
+	and record_warning.
+	* locale/programs/ld-numeric.c
+	(numeric_finish): Use record_error.
+	* locale/programs/ld-paper.c: Don't include error.h.
+	(paper_finish): Use record_error.
+	* locale/programs/ld-telephone.c: Don't include error.h.
+	(telephone_finish): Use record_error.
+	* locale/programs/ld-time.c (time_finish): Likewise.
+	* locale/programs/linereader.h (lr_error): Make inline func.
+	* locale/programs/localedef.c: Define recorded_warning_count,
+	and recorded_error_count.
+	(main): Use record_error. Use recorded_error_count and
+	recorded_warning_count to issue correct error returns.
+	(add_to_readlist): Use record_error.
+	(find_locale): Likewise.
+	(load_locale): Likewise.
+	* locale/programs/localedef.h: Remove be_quiet
+	and WITH_CUR_LOCALE.
+	* locale/programs/locarchive.c (compare_from_file): Use
+	record_error.
+	* locale/programs/locfile.c (write_locale_data): Use
+	record_error.
+	* locale/programs/repertoire.c: Dont include error.h.
+	(repertoire_complain): Use record_error.
+	* localedata/tst-fmon.sh: Expect failures from localedef.
+	* localedata/tst-locale.sh: Likewise.
+	* localedata/gen-locale.sh: Expect failures from SHIFT_JIS.
+
 2017-10-12  Carlos O'Donell  <carlos@redhat.com>
 
 	* localedata/unicode-gen/Makefile (GENERATED): Use i18n_ctype.
diff --git a/locale/programs/charmap-dir.c b/locale/programs/charmap-dir.c
index e55ab86e28..a9212b72fb 100644
--- a/locale/programs/charmap-dir.c
+++ b/locale/programs/charmap-dir.c
@@ -16,7 +16,6 @@
 
 #include <dirent.h>
 #include <errno.h>
-#include <error.h>
 #include <fcntl.h>
 #include <libintl.h>
 #include <spawn.h>
@@ -54,8 +53,9 @@ charmap_opendir (const char *directory)
   dir = opendir (directory);
   if (dir == NULL)
     {
-      WITH_CUR_LOCALE (error (1, errno, gettext ("\
-cannot read character map directory `%s'"), directory));
+      record_error (1, errno, gettext ("\
+cannot read character map directory `%s'"),
+		    directory);
       return NULL;
     }
 
diff --git a/locale/programs/charmap.c b/locale/programs/charmap.c
index 129aefffc1..a670db9532 100644
--- a/locale/programs/charmap.c
+++ b/locale/programs/charmap.c
@@ -26,7 +26,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <error.h>
 #include <stdint.h>
 
 #include "localedef.h"
@@ -135,8 +134,9 @@ charmap_read (const char *filename, int verbose, int error_not_found,
 	result = parse_charmap (cmfile, verbose, be_quiet);
 
       if (result == NULL && error_not_found)
-	WITH_CUR_LOCALE (error (0, errno, _("\
-character map file `%s' not found"), filename));
+	record_error (0, errno,
+		      _("character map file `%s' not found"),
+		      filename);
     }
 
   if (result == NULL && filename != NULL && strchr (filename, '/') == NULL)
@@ -192,8 +192,9 @@ character map file `%s' not found"), filename));
 	result = parse_charmap (cmfile, verbose, be_quiet);
 
       if (result == NULL)
-	WITH_CUR_LOCALE (error (4, errno, _("\
-default character map file `%s' not found"), DEFAULT_CHARMAP));
+	record_error (4, errno,
+		      _("default character map file `%s' not found"),
+		      DEFAULT_CHARMAP);
     }
 
   if (result != NULL && result->code_set_name == NULL)
@@ -255,9 +256,9 @@ default character map file `%s' not found"), DEFAULT_CHARMAP));
 
       if (failed)
 	{
-	  WITH_CUR_LOCALE (fprintf (stderr, _("\
+	  record_warning (_("\
 character map `%s' is not ASCII compatible, locale not ISO C compliant\n"),
-				    result->code_set_name));
+			  result->code_set_name);
 	  enc_not_ascii_compatible = true;
 	}
     }
@@ -333,10 +334,9 @@ parse_charmap (struct linereader *cmfile, int verbose, int be_quiet)
 		result->mb_cur_min = result->mb_cur_max;
 	      if (result->mb_cur_min > result->mb_cur_max)
 		{
-		  if (!be_quiet)
-		    WITH_CUR_LOCALE (error (0, 0, _("\
+		  record_error (0, 0, _("\
 %s: <mb_cur_max> must be greater than <mb_cur_min>\n"),
-					    cmfile->fname));
+				cmfile->fname);
 
 		  result->mb_cur_min = result->mb_cur_max;
 		}
@@ -395,11 +395,10 @@ parse_charmap (struct linereader *cmfile, int verbose, int be_quiet)
 	      if (arg->tok != tok_number)
 		goto badarg;
 
-	      if (verbose
-		  && ((nowtok == tok_mb_cur_max
+	      if ((nowtok == tok_mb_cur_max
 		       && result->mb_cur_max != 0)
 		      || (nowtok == tok_mb_cur_max
-			  && result->mb_cur_max != 0)))
+			  && result->mb_cur_max != 0))
 		lr_error (cmfile, _("duplicate definition of <%s>"),
 			  nowtok == tok_mb_cur_min
 			  ? "mb_cur_min" : "mb_cur_max");
@@ -839,16 +838,16 @@ only WIDTH definitions are allowed to follow the CHARMAP definition"));
 	  continue;
 
 	default:
-	  WITH_CUR_LOCALE (error (5, 0, _("%s: error in state machine"),
-				  __FILE__));
+	  record_error (5, 0, _("%s: error in state machine"),
+			__FILE__);
 	  /* NOTREACHED */
 	}
       break;
     }
 
-  if (state != 91 && !be_quiet)
-    WITH_CUR_LOCALE (error (0, 0, _("%s: premature end of file"),
-			    cmfile->fname));
+  if (state != 91)
+    record_error (0, 0, _("%s: premature end of file"),
+		  cmfile->fname);
 
   lr_close (cmfile);
 
diff --git a/locale/programs/ld-address.c b/locale/programs/ld-address.c
index 2488a5ce5c..a2892c71a2 100644
--- a/locale/programs/ld-address.c
+++ b/locale/programs/ld-address.c
@@ -20,7 +20,6 @@
 #endif
 
 #include <byteswap.h>
-#include <error.h>
 #include <langinfo.h>
 #include <string.h>
 #include <stdint.h>
@@ -131,9 +130,8 @@ address_finish (struct localedef_t *locale, const struct charmap_t *charmap)
 	 empty one.  */
       if (address == NULL)
 	{
-	  if (! be_quiet)
-	    WITH_CUR_LOCALE (error (0, 0, _("\
-No definition for %s category found"), "LC_ADDRESS"));
+	  record_warning (_("\
+No definition for %s category found"), "LC_ADDRESS");
 	  address_startup (NULL, locale, 0);
 	  address = locale->categories[LC_ADDRESS].address;
 	  nothing = 1;
@@ -143,8 +141,8 @@ No definition for %s category found"), "LC_ADDRESS"));
   if (address->postal_fmt == NULL)
     {
       if (! nothing)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),
-				"LC_ADDRESS", "postal_fmt"));
+	record_error (0, 0, _("%s: field `%s' not defined"),
+		      "LC_ADDRESS", "postal_fmt");
       /* Use as the default value the value of the i18n locale.  */
       address->postal_fmt = "%a%N%f%N%d%N%b%N%s %h %e %r%N%C-%z %T%N%c%N";
     }
@@ -155,8 +153,8 @@ No definition for %s category found"), "LC_ADDRESS"));
       const char *cp = address->postal_fmt;
 
       if (*cp == '\0')
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' must not be empty"),
-				"LC_ADDRESS", "postal_fmt"));
+	record_error (0, 0, _("%s: field `%s' must not be empty"),
+		      "LC_ADDRESS", "postal_fmt");
       else
 	while (*cp != '\0')
 	  {
@@ -167,9 +165,9 @@ No definition for %s category found"), "LC_ADDRESS"));
 		  ++cp;
 		if (strchr ("nafdbshNtreClzTSc%", *cp) == NULL)
 		  {
-		    WITH_CUR_LOCALE (error (0, 0, _("\
+		    record_error (0, 0, _("\
 %s: invalid escape `%%%c' sequence in field `%s'"),
-					    "LC_ADDRESS", *cp, "postal_fmt"));
+				  "LC_ADDRESS", *cp, "postal_fmt");
 		    break;
 		  }
 	      }
@@ -181,8 +179,7 @@ No definition for %s category found"), "LC_ADDRESS"));
   if (address->cat == NULL)						      \
     {									      \
       if (verbose && ! nothing)						      \
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),	      \
-				"LC_ADDRESS", #cat));  	    		      \
+	record_warning (_("%s: field `%s' not defined"), "LC_ADDRESS", #cat); \
       address->cat = "";						      \
     }
 
@@ -199,16 +196,16 @@ No definition for %s category found"), "LC_ADDRESS"));
   if (address->lang_term == NULL)
     {
       if (verbose && ! nothing)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),
-				"LC_ADDRESS", "lang_term"));
+	record_warning (_("%s: field `%s' not defined"), "LC_ADDRESS",
+			"lang_term");
       address->lang_term = "";
       cnt = sizeof (iso639) / sizeof (iso639[0]);
     }
   else if (address->lang_term[0] == '\0')
     {
       if (verbose)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' must not be empty"),
-				"LC_ADDRESS", "lang_term"));
+	record_warning (_("%s: field `%s' must not be empty"), "LC_ADDRESS",
+			"lang_term");
       cnt = sizeof (iso639) / sizeof (iso639[0]);
     }
   else
@@ -218,9 +215,9 @@ No definition for %s category found"), "LC_ADDRESS"));
 	if (strcmp (address->lang_term, iso639[cnt].term) == 0)
 	  break;
       if (cnt == sizeof (iso639) / sizeof (iso639[0]))
-	WITH_CUR_LOCALE (error (0, 0, _("\
+	record_error (0, 0, _("\
 %s: terminology language code `%s' not defined"),
-				"LC_ADDRESS", address->lang_term));
+		      "LC_ADDRESS", address->lang_term);
     }
 
   if (address->lang_ab == NULL)
@@ -228,8 +225,8 @@ No definition for %s category found"), "LC_ADDRESS"));
       if ((cnt == sizeof (iso639) / sizeof (iso639[0])
 	   || iso639[cnt].ab[0] != '\0')
 	  && verbose && ! nothing)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),
-				"LC_ADDRESS", "lang_ab"));
+	record_warning (_("%s: field `%s' not defined"), "LC_ADDRESS",
+			"lang_ab");
       address->lang_ab = "";
     }
   else if (address->lang_ab[0] == '\0')
@@ -237,14 +234,14 @@ No definition for %s category found"), "LC_ADDRESS"));
       if ((cnt == sizeof (iso639) / sizeof (iso639[0])
 	   || iso639[cnt].ab[0] != '\0')
 	  && verbose)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' must not be empty"),
-				"LC_ADDRESS", "lang_ab"));
+	record_warning (_("%s: field `%s' must not be empty"),
+			"LC_ADDRESS", "lang_ab");
     }
   else if (cnt < sizeof (iso639) / sizeof (iso639[0])
 	   && iso639[cnt].ab[0] == '\0')
     {
-      WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' must not be defined"),
-			      "LC_ADDRESS", "lang_ab"));
+      record_error (0, 0, _("%s: field `%s' must not be defined"),
+		    "LC_ADDRESS", "lang_ab");
 
       address->lang_ab = "";
     }
@@ -257,16 +254,16 @@ No definition for %s category found"), "LC_ADDRESS"));
 	    if (strcmp (address->lang_ab, iso639[cnt].ab) == 0)
 	      break;
 	  if (cnt == sizeof (iso639) / sizeof (iso639[0]))
-	    WITH_CUR_LOCALE (error (0, 0, _("\
+	    record_error (0, 0, _("\
 %s: language abbreviation `%s' not defined"),
-				    "LC_ADDRESS", address->lang_ab));
+			  "LC_ADDRESS", address->lang_ab);
 	}
       else
 	if (strcmp (iso639[cnt].ab, address->lang_ab) != 0
 	    && iso639[cnt].ab[0] != '\0')
-	  WITH_CUR_LOCALE (error (0, 0, _("\
+	  record_error (0, 0, _("\
 %s: `%s' value does not match `%s' value"),
-				  "LC_ADDRESS", "lang_ab", "lang_term"));
+			"LC_ADDRESS", "lang_ab", "lang_term");
     }
 
   if (address->lang_lib == NULL)
@@ -275,8 +272,8 @@ No definition for %s category found"), "LC_ADDRESS"));
   else if (address->lang_lib[0] == '\0')
     {
       if (verbose)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' must not be empty"),
-				"LC_ADDRESS", "lang_lib"));
+	record_warning (_("%s: field `%s' must not be empty"),
+			"LC_ADDRESS", "lang_lib");
     }
   else
     {
@@ -286,22 +283,22 @@ No definition for %s category found"), "LC_ADDRESS"));
 	    if (strcmp (address->lang_lib, iso639[cnt].lib) == 0)
 	      break;
 	  if (cnt == sizeof (iso639) / sizeof (iso639[0]))
-	    WITH_CUR_LOCALE (error (0, 0, _("\
+	    record_error (0, 0, _("\
 %s: language abbreviation `%s' not defined"),
-				    "LC_ADDRESS", address->lang_lib));
+			  "LC_ADDRESS", address->lang_lib);
 	}
       else
 	if (strcmp (iso639[cnt].ab, address->lang_ab) != 0)
-	  WITH_CUR_LOCALE (error (0, 0, _("\
+	  record_error (0, 0, _("\
 %s: `%s' value does not match `%s' value"), "LC_ADDRESS", "lang_lib",
-				  helper == 1 ? "lang_term" : "lang_ab"));
+			helper == 1 ? "lang_term" : "lang_ab");
     }
 
   if (address->country_num == 0)
     {
       if (verbose && ! nothing)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),
-				"LC_ADDRESS", "country_num"));
+	record_warning (_("%s: field `%s' not defined"), "LC_ADDRESS",
+			"country_num");
       cnt = sizeof (iso3166) / sizeof (iso3166[0]);
     }
   else
@@ -311,36 +308,35 @@ No definition for %s category found"), "LC_ADDRESS"));
 	  break;
 
       if (cnt == sizeof (iso3166) / sizeof (iso3166[0]))
-	WITH_CUR_LOCALE (error (0, 0, _("\
+	record_error (0, 0, _("\
 %s: numeric country code `%d' not valid"),
-				"LC_ADDRESS", address->country_num));
+		      "LC_ADDRESS", address->country_num);
     }
 
   if (address->country_ab2 == NULL)
     {
       if (verbose && ! nothing)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),
-				"LC_ADDRESS", "country_ab2"));
+	record_warning (_("%s: field `%s' not defined"), "LC_ADDRESS",
+			"country_ab2");
       address->country_ab2 = "  ";
     }
   else if (cnt != sizeof (iso3166) / sizeof (iso3166[0])
 	   && strcmp (address->country_ab2, iso3166[cnt].ab2) != 0)
-    WITH_CUR_LOCALE (error (0, 0,
-			    _("%s: `%s' value does not match `%s' value"),
-			    "LC_ADDRESS", "country_ab2", "country_num"));
+    record_error (0, 0, _("%s: `%s' value does not match `%s' value"),
+		  "LC_ADDRESS", "country_ab2", "country_num");
 
   if (address->country_ab3 == NULL)
     {
       if (verbose && ! nothing)
-	WITH_CUR_LOCALE (error (0, 0, _("%s: field `%s' not defined"),
-				"LC_ADDRESS", "country_ab3"));
+	record_warning (_("%s: field `%s' not defined"), "LC_ADDRESS",
+			"country_ab3");
       address->country_ab3 = "   ";
     }
   else if (cnt != sizeof (iso3166) / sizeof (iso3166[0])
 	   && strcmp (address->country_ab3, iso3166[cnt].ab3) != 0)
-    WITH_CUR_LOCALE (error (0, 0, _("\
+    record_error (0, 0, _("\
 %s: `%s' value does not match `%s' value"),
-			    "LC_ADDRESS", "country_ab3", "country_num"));
+		  "LC_ADDRESS", "country_ab3", "country_num");
 }
 
 
diff --git a/locale/programs/ld-collate.c b/locale/programs/ld-collate.c
index