maint: consolidate developer debug messages

Both factor and numfmt recently introduced debug messages
for developers, enabled by --verbose and ---devdebug respectively.
There were a few issues though:
 1. They used different mechanisms to enable these messages.
 2. factor used --verbose which might be needed for something else
 3. They used different methods to output the messages,
    and numfmt used error() which added an unwanted newline
 4. numfmt marked all these messages for translation and factor
    marked a couple.  We really don't need these translated.
So we fix the above issues here while renaming the enabling
option for both commands to ---debug (still undocumented).

* src/factor.c (verbose): Rename to dev_debug and change from int to
bool as it's just a toggle flag.
(long_options): Rename --verbose to ---debug.
* src/system.h (devmsg): A new inline function to output a message
if enabled by a global dev_debug variable in the compilation unit.
* src/numfmt.c: Use devmsg() rather than error().
Also remove the translation tags from these messages.
Also change debug flag to bool from int.
* tests/misc/numfmt.pl: Adjust for the ---devdebug to ---debug change.
* cfg.mk (sc_marked_devdiagnostics): Add a syntax check to ensure
translations are not added to devmsg calls.

Reported by Göran Uddeborg in http://bugs.gnu.org/13665

5 files changed, 83 insertions, 102 deletions

diff --git a/cfg.mk b/cfg.mk
index fbc64b4af..77356ab13 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -521,6 +521,13 @@ sc_THANKS_in_duplicates:
 	    && { echo '$(ME): remove the above names from THANKS.in'	\
 		  1>&2; exit 1; } || :
 
+# Look for developer diagnostics that are marked for translation.
+# This won't find any for which devmsg's format string is on a separate line.
+sc_marked_devdiagnostics:
+	@prohibit='\<devmsg *\(.*_\('                                   \
+	halt='found marked developer diagnostic(s)'                     \
+	  $(_sc_search_regexp)
+
 # Override the default Cc: used in generating an announcement.
 announcement_Cc_ = $(translation_project_), \
   coreutils@gnu.org, coreutils-announce@gnu.org
diff --git a/src/factor.c b/src/factor.c
index 95451a58d..df3d7a0a9 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -216,12 +216,12 @@ static enum alg_type alg;
 
 enum
 {
-  VERBOSE_OPTION = CHAR_MAX + 1
+  DEV_DEBUG_OPTION = CHAR_MAX + 1
 };
 
 static struct option const long_options[] =
 {
-  {"verbose", no_argument, NULL, VERBOSE_OPTION},
+  {"-debug", no_argument, NULL, DEV_DEBUG_OPTION},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -685,8 +685,9 @@ static const struct primes_dtab primes_dtab[] = {
    the integers used to generate primes.h.  */
 verify (W <= WIDE_UINT_BITS);
 
-/* This flag is honored only in the GMP code. */
-static int verbose = 0;
+/* debugging for developers.  Enables devmsg().
+   This flag is used only in the GMP code.  */
+bool dev_debug = false;
 
 /* Prove primality or run probabilistic tests.  */
 static bool flag_prove_primality = true;
@@ -703,18 +704,6 @@ static bool flag_prove_primality = true;
 #endif
 
 static void
-debug (char const *fmt, ...)
-{
-  if (verbose)
-    {
-      va_list ap;
-      va_start (ap, fmt);
-      vfprintf (stderr, fmt, ap);
-      va_end (ap);
-    }
-}
-
-static void
 factor_insert_refind (struct factors *factors, uintmax_t p, unsigned int i,
                       unsigned int off)
 {
@@ -844,7 +833,7 @@ mp_factor_using_division (mpz_t t, struct mp_factors *factors)
   mpz_t q;
   unsigned long int p;
 
-  debug ("[trial division] ");
+  devmsg ("[trial division] ");
 
   mpz_init (q);
 
@@ -1665,7 +1654,7 @@ mp_factor_using_pollard_rho (mpz_t n, unsigned long int a,
   mpz_t x, z, y, P;
   mpz_t t, t2;
 
-  debug ("[pollard-rho (%lu)] ", a);
+  devmsg ("[pollard-rho (%lu)] ", a);
 
   mpz_inits (t, t2, NULL);
   mpz_init_set_si (y, 2);
@@ -1728,7 +1717,7 @@ mp_factor_using_pollard_rho (mpz_t n, unsigned long int a,
 
       if (!mp_prime_p (t))
         {
-          debug ("[composite factor--restarting pollard-rho] ");
+          devmsg ("[composite factor--restarting pollard-rho] ");
           mp_factor_using_pollard_rho (t, a + 1, factors);
         }
       else
@@ -2247,7 +2236,7 @@ mp_factor (mpz_t t, struct mp_factors *factors)
 
       if (mpz_cmp_ui (t, 1) != 0)
         {
-          debug ("[is number prime?] ");
+          devmsg ("[is number prime?] ");
           if (mp_prime_p (t))
             mp_factor_insert (factors, t);
           else
@@ -2400,7 +2389,7 @@ print_factors (const char *input)
     case LONGINT_OK:
       if (((t1 << 1) >> 1) == t1)
         {
-          debug ("[%s]", _("using single-precision arithmetic"));
+          devmsg ("[using single-precision arithmetic] ");
           print_factors_single (t1, t0);
           return true;
         }
@@ -2416,7 +2405,7 @@ print_factors (const char *input)
     }
 
 #if HAVE_GMP
-  debug ("[%s]", _("using arbitrary-precision arithmetic"));
+  devmsg ("[using arbitrary-precision arithmetic] ");
   mpz_t t;
   struct mp_factors factors;
 
@@ -2502,8 +2491,8 @@ main (int argc, char **argv)
     {
       switch (c)
         {
-        case VERBOSE_OPTION:
-          verbose = 1;
+        case DEV_DEBUG_OPTION:
+          dev_debug = true;
           break;
 
         case 's':
diff --git a/src/numfmt.c b/src/numfmt.c
index e42cbf86b..f7c8e5ede 100644
--- a/src/numfmt.c
+++ b/src/numfmt.c
@@ -136,7 +136,7 @@ static struct option const longopts[] =
   {"delimiter", required_argument, NULL, 'd'},
   {"field", required_argument, NULL, FIELD_OPTION},
   {"debug", no_argument, NULL, DEBUG_OPTION},
-  {"-devdebug", no_argument, NULL, DEV_DEBUG_OPTION},
+  {"-debug", no_argument, NULL, DEV_DEBUG_OPTION},
   {"header", optional_argument, NULL, HEADER_OPTION},
   {"format", required_argument, NULL, FORMAT_OPTION},
   {"invalid", required_argument, NULL, INVALID_OPTION},
@@ -188,10 +188,10 @@ static uintmax_t header = 0;
 
 /* Debug for users: print warnings to STDERR about possible
    error (similar to sort's debug).  */
-static int debug = 0;
+static bool debug;
 
-/* debugging for developers - to be removed in final version?  */
-static int dev_debug = 0;
+/* debugging for developers.  Enables devmsg().  */
+bool dev_debug = false;
 
 /* will be set according to the current locale.  */
 static const char *decimal_point;
@@ -583,18 +583,16 @@ simple_strtod_human (const char *input_str,
   /* 'scale_auto' is checked below.  */
   int scale_base = default_scale_base (allowed_scaling);
 
-  if (dev_debug)
-    error (0, 0, _("simple_strtod_human:\n  input string: '%s'\n  "
-                   "locale decimal-point: '%s'\n"), input_str, decimal_point);
+  devmsg ("simple_strtod_human:\n  input string: '%s'\n  "
+          "locale decimal-point: '%s'\n", input_str, decimal_point);
 
   enum simple_strtod_error e =
     simple_strtod_float (input_str, endptr, value, precision);
   if (e != SSE_OK && e != SSE_OK_PRECISION_LOSS)
     return e;
 
-  if (dev_debug)
-    error (0, 0, _("  parsed numeric value: %Lf\n"
-                   "  input precision = %d\n"), *value, (int)*precision);
+  devmsg ("  parsed numeric value: %Lf\n"
+          "  input precision = %d\n", *value, (int)*precision);
 
   if (**endptr != '\0')
     {
@@ -619,9 +617,8 @@ simple_strtod_human (const char *input_str,
               is followed by an 'i' (e.g. Ki, Mi, Gi).  */
           scale_base = 1024;
           (*endptr)++;              /* skip second  ('i') suffix character.  */
-          if (dev_debug)
-            error (0, 0, _("  Auto-scaling, found 'i', switching to base %d\n"),
-                    scale_base);
+          devmsg ("  Auto-scaling, found 'i', switching to base %d\n",
+                  scale_base);
         }
 
       *precision = 0;  /* Reset, to select precision based on scale.  */
@@ -637,15 +634,12 @@ simple_strtod_human (const char *input_str,
 
   long double multiplier = powerld (scale_base, power);
 
-  if (dev_debug)
-    error (0, 0, _("  suffix power=%d^%d = %Lf\n"),
-           scale_base, power, multiplier);
+  devmsg ("  suffix power=%d^%d = %Lf\n", scale_base, power, multiplier);
 
   /* TODO: detect loss of precision and overflows.  */
   (*value) = (*value) * multiplier;
 
-  if (dev_debug)
-    error (0, 0, _("  returning value: %Lf (%LG)\n"), *value, *value);
+  devmsg ("  returning value: %Lf (%LG)\n", *value, *value);
 
   return e;
 }
@@ -695,8 +689,7 @@ double_to_human (long double val, int precision,
                  char *buf, size_t buf_size,
                  enum scale_type scale, int group, enum round_type round)
 {
-  if (dev_debug)
-    error (0, 0, _("double_to_human:\n"));
+  devmsg ("double_to_human:\n");
 
   if (scale == scale_none)
     {
@@ -704,11 +697,9 @@ double_to_human (long double val, int precision,
       val = simple_round (val, round);
       val /= powerld (10, precision);
 
-      if (dev_debug)
-        error (0, 0,
-               (group) ?
-               _("  no scaling, returning (grouped) value: %'.*Lf\n") :
-               _("  no scaling, returning value: %.*Lf\n"), precision, val);
+      devmsg ((group) ?
+              "  no scaling, returning (grouped) value: %'.*Lf\n" :
+              "  no scaling, returning value: %.*Lf\n", precision, val);
 
       int i = snprintf (buf, buf_size, (group) ? "%'.*Lf" : "%.*Lf",
                         precision, val);
@@ -724,9 +715,7 @@ double_to_human (long double val, int precision,
   /* Normalize val to scale. */
   unsigned int power = 0;
   val = expld (val, scale_base, &power);
-  if (dev_debug)
-    error (0, 0, _("  scaled value to %Lf * %0.f ^ %d\n"),
-           val, scale_base, power);
+  devmsg ("  scaled value to %Lf * %0.f ^ %d\n", val, scale_base, power);
 
   /* Perform rounding. */
   int ten_or_less = 0;
@@ -754,9 +743,7 @@ double_to_human (long double val, int precision,
   int show_decimal_point = (val != 0) && (absld (val) < 10) && (power > 0);
   /* && (absld (val) > simple_round_floor (val))) */
 
-  if (dev_debug)
-    error (0, 0, _("  after rounding, value=%Lf * %0.f ^ %d\n"),
-           val, scale_base, power);
+  devmsg ("  after rounding, value=%Lf * %0.f ^ %d\n", val, scale_base, power);
 
   snprintf (buf, buf_size, (show_decimal_point) ? "%.1Lf%s" : "%.0Lf%s",
             val, suffix_power_character (power));
@@ -764,8 +751,7 @@ double_to_human (long double val, int precision,
   if (scale == scale_IEC_I && power > 0)
     strncat (buf, "i", buf_size - strlen (buf) - 1);
 
-  if (dev_debug)
-    error (0, 0, _("  returning value: '%s'\n"), buf);
+  devmsg ("  returning value: '%s'\n", buf);
 
   return;
 }
@@ -1012,14 +998,13 @@ parse_format_string (char const *fmt)
                strlen (fmt + suffix_pos));
     }
 
-  if (dev_debug)
-    error (0, 0, _("format String:\n  input: %s\n  grouping: %s\n"
+  devmsg ("format String:\n  input: %s\n  grouping: %s\n"
                    "  padding width: %ld\n  alignment: %s\n"
-                   "  prefix: '%s'\n  suffix: '%s'\n"),
-           quote (fmt), (grouping) ? "yes" : "no",
-           padding_width,
-           (padding_alignment == MBS_ALIGN_LEFT) ? "Left" : "Right",
-           format_str_prefix, format_str_suffix);
+                   "  prefix: '%s'\n  suffix: '%s'\n",
+          quote (fmt), (grouping) ? "yes" : "no",
+          padding_width,
+          (padding_alignment == MBS_ALIGN_LEFT) ? "Left" : "Right",
+          format_str_prefix, format_str_suffix);
 }
 
 /* Parse a numeric value (with optional suffix) from a string.
@@ -1087,10 +1072,7 @@ prepare_padded_number (const long double val, size_t precision)
   if (suffix)
     strncat (buf, suffix, sizeof (buf) - strlen (buf) -1);
 
-  if (dev_debug)
-    error (0, 0, _("formatting output:\n  value: %Lf\n  humanized: '%s'\n"),
-           val, buf);
-
+  devmsg ("formatting output:\n  value: %Lf\n  humanized: '%s'\n", val, buf);
 
   if (padding_width && strlen (buf) < padding_width)
     {
@@ -1098,9 +1080,7 @@ prepare_padded_number (const long double val, size_t precision)
       mbsalign (buf, padding_buffer, padding_buffer_size, &w,
                 padding_alignment, MBA_UNIBYTE_ONLY);
 
-      if (dev_debug)
-        error (0, 0, _("  After padding: '%s'\n"), padding_buffer);
-
+      devmsg ("  After padding: '%s'\n", padding_buffer);
     }
   else
     {
@@ -1136,14 +1116,10 @@ process_suffixed_number (char *text, long double *result, size_t *precision)
         {
           /* trim suffix, ONLY if it's at the end of the text.  */
           *possible_suffix = '\0';
-          if (dev_debug)
-            error (0, 0, _("trimming suffix '%s'\n"), suffix);
+          devmsg ("trimming suffix '%s'\n", suffix);
         }
       else
-        {
-          if (dev_debug)
-            error (0, 0, _("no valid suffix found\n"));
-        }
+        devmsg ("no valid suffix found\n");
     }
 
   /* Skip white space - always.  */
@@ -1164,9 +1140,7 @@ process_suffixed_number (char *text, long double *result, size_t *precision)
         {
           padding_width = 0;
         }
-      if (dev_debug)
-        error (0, 0, _("setting Auto-Padding to %ld characters\n"),
-               padding_width);
+     devmsg ("setting Auto-Padding to %ld characters\n", padding_width);
     }
 
   long double val = 0;
@@ -1233,9 +1207,7 @@ extract_fields (char *line, int _field,
   *_data = NULL;
   *_suffix = NULL;
 
-  if (dev_debug)
-    error (0, 0, _("extracting Fields:\n  input: '%s'\n  field: %d\n"),
-           line, _field);
+  devmsg ("extracting Fields:\n  input: '%s'\n  field: %d\n", line, _field);
 
   if (field > 1)
     {
@@ -1245,8 +1217,7 @@ extract_fields (char *line, int _field,
       if (*ptr == '\0')
         {
           /* not enough fields in the input - print warning?  */
-          if (dev_debug)
-            error (0, 0, _("  TOO FEW FIELDS!\n  prefix: '%s'\n"), *_prefix);
+          devmsg ("  TOO FEW FIELDS!\n  prefix: '%s'\n", *_prefix);
           return;
         }
 
@@ -1266,9 +1237,8 @@ extract_fields (char *line, int _field,
   else
     *_suffix = NULL;
 
-  if (dev_debug)
-    error (0, 0, _("  prefix: '%s'\n  number: '%s'\n  suffix: '%s'\n"),
-           *_prefix, *_data, *_suffix);
+  devmsg ("  prefix: '%s'\n  number: '%s'\n  suffix: '%s'\n",
+          *_prefix, *_data, *_suffix);
 }
 
 
@@ -1409,12 +1379,12 @@ main (int argc, char **argv)
           break;
 
         case DEBUG_OPTION:
-          debug = 1;
+          debug = true;
           break;
 
         case DEV_DEBUG_OPTION:
-          dev_debug = 1;
-          debug = 1;
+          dev_debug = true;
+          debug = true;
           break;
 
         case HEADER_OPTION:
diff --git a/src/system.h b/src/system.h
index 1677999ef..6c310ad2c 100644
--- a/src/system.h
+++ b/src/system.h
@@ -649,6 +649,21 @@ stzncpy (char *restrict dest, char const *restrict src, size_t len)
   return dest;
 }
 
+/* Like error(0, 0, ...), but without an implicit newline.
+   Also a noop unless the global DEV_DEBUG is set.  */
+extern bool dev_debug;
+static inline void
+devmsg (char const *fmt, ...)
+{
+  if (dev_debug)
+    {
+      va_list ap;
+      va_start (ap, fmt);
+      vfprintf (stderr, fmt, ap);
+      va_end (ap);
+    }
+}
+
 #ifndef ARRAY_CARDINALITY
 # define ARRAY_CARDINALITY(Array) (sizeof (Array) / sizeof *(Array))
 #endif
diff --git a/tests/misc/numfmt.pl b/tests/misc/numfmt.pl
index b46e4d55b..61917fb8a 100644
--- a/tests/misc/numfmt.pl
+++ b/tests/misc/numfmt.pl
@@ -643,37 +643,37 @@ my @Tests =
 
      # dev-debug messages - the actual messages don't matter
      # just ensure the program works, and for code coverage testing.
-     ['devdebug-1', '---devdebug --from=si 4.9K', {OUT=>"4900"},
+     ['devdebug-1', '---debug --from=si 4.9K', {OUT=>"4900"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-2', '---devdebug 4900', {OUT=>"4900"},
+     ['devdebug-2', '---debug 4900', {OUT=>"4900"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-3', '---devdebug --from=auto 4Mi', {OUT=>"4194304"},
+     ['devdebug-3', '---debug --from=auto 4Mi', {OUT=>"4194304"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-4', '---devdebug --to=si 4000000', {OUT=>"4.0M"},
+     ['devdebug-4', '---debug --to=si 4000000', {OUT=>"4.0M"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-5', '---devdebug --to=si --padding=5 4000000', {OUT=>" 4.0M"},
+     ['devdebug-5', '---debug --to=si --padding=5 4000000', {OUT=>" 4.0M"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-6', '---devdebug --suffix=Foo 1234Foo', {OUT=>"1234Foo"},
+     ['devdebug-6', '---debug --suffix=Foo 1234Foo', {OUT=>"1234Foo"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-7', '---devdebug --suffix=Foo 1234', {OUT=>"1234Foo"},
+     ['devdebug-7', '---debug --suffix=Foo 1234', {OUT=>"1234Foo"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-9', '---devdebug --grouping 10000', {OUT=>"10000"},
+     ['devdebug-9', '---debug --grouping 10000', {OUT=>"10000"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-10', '---devdebug --format %f 10000', {OUT=>"10000"},
+     ['devdebug-10', '---debug --format %f 10000', {OUT=>"10000"},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-11', '---devdebug --format "%\'-10f" 10000',{OUT=>"10000     "},
+     ['devdebug-11', '---debug --format "%\'-10f" 10000',{OUT=>"10000     "},
              {ERR=>""},
              {ERR_SUBST=>"s/.*//msg"}],
-     ['devdebug-12', '---devdebug --field 2 A',{OUT=>""},
+     ['devdebug-12', '---debug --field 2 A',{OUT=>""},
              {ERR=>""}, {EXIT=>2},
              {ERR_SUBST=>"s/.*//msg"}],