[libgfortran,patch] Remove never-used debugging code

[libgfortran,patch] Remove never-used debugging code

[FX]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

Continuing the summer clean-ups.

There is in the init() function some dead debugging code, which as far as I know is never used: it’s compiled conditionaly on DEBUG being defined, and but we don’t have anything that can define DEBUG. The commented-out “resume” functionality was never implemented in gfortran. And show_variables() isn’t very useful.

OK to commit to trunk?

FX



[-- Attachment #2: unusedcode.ChangeLog --]
[-- Type: application/octet-stream, Size: 216 bytes --]

2015-08-24  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

	* libgfortran.h (show_variables): Remove prototype.
	* runtime/environ.c (show_variables): Remove function.
	* runtime/main.c (init): Remove dead code.


[-- Attachment #3: unusedcode.diff --]
[-- Type: application/octet-stream, Size: 2551 bytes --]

Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 227127)
+++ libgfortran/libgfortran.h	(working copy)
@@ -796,9 +796,6 @@ internal_proto(check_buffered);
 extern void init_variables (void);
 internal_proto(init_variables);
 
-extern void show_variables (void);
-internal_proto(show_variables);
-
 unit_convert get_unformatted_convert (int);
 internal_proto(get_unformatted_convert);
 
Index: libgfortran/runtime/environ.c
===================================================================
--- libgfortran/runtime/environ.c	(revision 227127)
+++ libgfortran/runtime/environ.c	(working copy)
@@ -355,53 +355,6 @@ init_variables (void)
 }
 
 
-void
-show_variables (void)
-{
-  variable *v;
-  int n;
-
-  /* TODO: print version number.  */
-  estr_write ("GNU Fortran runtime library version "
-	     "UNKNOWN" "\n\n");
-
-  estr_write ("Environment variables:\n");
-  estr_write ("----------------------\n");
-
-  for (v = variable_table; v->name; v++)
-    {
-      n = estr_write (v->name);
-      print_spaces (25 - n);
-
-      if (v->show == show_integer)
-	estr_write ("Integer ");
-      else if (v->show == show_boolean)
-	estr_write ("Boolean ");
-      else
-	estr_write ("String  ");
-
-      v->show (v);
-      estr_write (v->desc);
-      estr_write ("\n\n");
-    }
-
-  /* System error codes */
-
-  estr_write ("\nRuntime error codes:");
-  estr_write ("\n--------------------\n");
-
-  for (n = LIBERROR_FIRST + 1; n < LIBERROR_LAST; n++)
-    if (n < 0 || n > 9)
-      st_printf ("%d  %s\n", n, translate_error (n));
-    else
-      st_printf (" %d  %s\n", n, translate_error (n));
-
-  estr_write ("\nCommand line arguments:\n");
-  estr_write ("  --help               Print this list\n");
-
-  exit (0);
-}
-
 /* This is the handling of the GFORTRAN_CONVERT_UNITS environment variable.
    It is called from environ.c to parse this variable, and from
    open.c to determine if the user specified a default for an
Index: libgfortran/runtime/main.c
===================================================================
--- libgfortran/runtime/main.c	(revision 227127)
+++ libgfortran/runtime/main.c	(working copy)
@@ -120,15 +120,6 @@ init (void)
 
   init_compile_options ();
 
-#ifdef DEBUG
-  /* Check for special command lines.  */
-
-  if (argc > 1 && strcmp (argv[1], "--help") == 0)
-    show_variables ();
-
-  /* if (argc > 1 && strcmp(argv[1], "--resume") == 0) resume();  */
-#endif
-
   random_seed_i4 (NULL, NULL, NULL);
 }
 

Re: [libgfortran,patch] Remove never-used debugging code

[FX]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

Turns out I missed some of the dead code. And I now also fixed comments and some formatting.
libgfortran/runtime/environ.c is now much more readable than before.
The patch is still a no-op, in terms of user functionality.
OK to commit to trunk?

FX



[-- Attachment #2: unusedcode.ChangeLog --]
[-- Type: application/octet-stream, Size: 279 bytes --]

2015-08-25  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

	* libgfortran.h (show_variables): Remove prototype.
	* runtime/environ.c (print_spaces, var_source, show_integer,
	show_boolean, show_sep, show_string): Remove functions.
	* runtime/main.c (init): Remove dead code.


[-- Attachment #3: unusedcode.diff --]
[-- Type: application/octet-stream, Size: 12346 bytes --]

Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 227174)
+++ libgfortran/libgfortran.h	(working copy)
@@ -793,9 +793,6 @@ internal_proto(xrealloc);
 extern void init_variables (void);
 internal_proto(init_variables);
 
-extern void show_variables (void);
-internal_proto(show_variables);
-
 unit_convert get_unformatted_convert (int);
 internal_proto(get_unformatted_convert);
 
Index: libgfortran/runtime/environ.c
===================================================================
--- libgfortran/runtime/environ.c	(revision 227174)
+++ libgfortran/runtime/environ.c	(working copy)
@@ -33,32 +33,8 @@ see the files COPYING3 and COPYING.RUNTI
 #endif
 
 
-/* Environment scanner.  Examine the environment for controlling minor
- * aspects of the program's execution.  Our philosophy here that the
- * environment should not prevent the program from running, so an
- * environment variable with a messed-up value will be interpreted in
- * the default way.
- *
- * Most of the environment is checked early in the startup sequence,
- * but other variables are checked during execution of the user's
- * program. */
-
-options_t options;
-
-
-typedef struct variable
-{
-  const char *name;
-  int value, *var;
-  void (*init) (struct variable *);
-  void (*show) (struct variable *);
-  const char *desc;
-  int bad;
-}
-variable;
-
-static void init_unformatted (variable *);
-
+/* Implementation of secure_getenv() for targets where it is not
+   provided. */
 
 #ifdef FALLBACK_SECURE_GETENV
 char *
@@ -72,43 +48,27 @@ secure_getenv (const char *name)
 #endif
 
 
-/* print_spaces()-- Print a particular number of spaces.  */
 
-static void
-print_spaces (int n)
-{
-  char buffer[80];
-  int i;
-
-  if (n <= 0)
-    return;
+/* Examine the environment for controlling aspects of the program's
+   execution.  Our philosophy here that the environment should not prevent
+   the program from running, so any invalid value will be ignored.  */
 
-  for (i = 0; i < n; i++)
-    buffer[i] = ' ';
-
-  buffer[i] = '\0';
-
-  estr_write (buffer);
-}
 
+options_t options;
 
-/* var_source()-- Return a string that describes where the value of a
- * variable comes from */
-
-static const char *
-var_source (variable * v)
+typedef struct variable
 {
-  if (getenv (v->name) == NULL)
-    return "Default";
-
-  if (v->bad)
-    return "Bad    ";
-
-  return "Set    ";
+  const char *name;
+  int default_value;
+  int *var;           
+  void (*init) (struct variable *);
 }
+variable;
+
+static void init_unformatted (variable *);
 
 
-/* init_integer()-- Initialize an integer environment variable.  */
+/* Initialize an integer environment variable.  */
 
 static void
 init_integer (variable * v)
@@ -117,26 +77,17 @@ init_integer (variable * v)
 
   p = getenv (v->name);
   if (p == NULL)
-    goto set_default;
+    return;
 
   for (q = p; *q; q++)
     if (!isdigit (*q) && (p != q || *q != '-'))
-      {
-	v->bad = 1;
-	goto set_default;
-      }
+      return;
 
   *v->var = atoi (p);
-  return;
-
- set_default:
-  *v->var = v->value;
-  return;
 }
 
 
-/* init_unsigned_integer()-- Initialize an integer environment variable
-   which has to be positive.  */
+/* Initialize an integer environment variable which has to be positive.  */
 
 static void
 init_unsigned_integer (variable * v)
@@ -145,35 +96,18 @@ init_unsigned_integer (variable * v)
 
   p = getenv (v->name);
   if (p == NULL)
-    goto set_default;
+    return;
 
   for (q = p; *q; q++)
     if (!isdigit (*q))
-      {
-	v->bad = 1;
-	goto set_default;
-      }
+      return;
 
   *v->var = atoi (p);
-  return;
-
- set_default:
-  *v->var = v->value;
-  return;
 }
 
 
-/* show_integer()-- Show an integer environment variable */
-
-static void
-show_integer (variable * v)
-{
-  st_printf ("%s  %d\n", var_source (v), *v->var);
-}
-
-
-/* init_boolean()-- Initialize a boolean environment variable.  We
- * only look at the first letter of the variable. */
+/* Initialize a boolean environment variable. We only look at the first
+   letter of the value. */
 
 static void
 init_boolean (variable * v)
@@ -182,36 +116,17 @@ init_boolean (variable * v)
 
   p = getenv (v->name);
   if (p == NULL)
-    goto set_default;
+    return;
 
   if (*p == '1' || *p == 'Y' || *p == 'y')
-    {
-      *v->var = 1;
-      return;
-    }
-
-  if (*p == '0' || *p == 'N' || *p == 'n')
-    {
-      *v->var = 0;
-      return;
-    }
-
-  v->bad = 1;
-
-set_default:
-  *v->var = v->value;
-  return;
+    *v->var = 1;
+  else if (*p == '0' || *p == 'N' || *p == 'n')
+    *v->var = 0;
 }
 
 
-/* show_boolean()-- Show a boolean environment variable */
-
-static void
-show_boolean (variable * v)
-{
-  st_printf ("%s  %s\n", var_source (v), *v->var ? "Yes" : "No");
-}
-
+/* Initialize a list output separator.  It may contain any number of spaces
+   and at most one comma.  */
 
 static void
 init_sep (variable * v)
@@ -223,7 +138,6 @@ init_sep (variable * v)
   if (p == NULL)
     goto set_default;
 
-  v->bad = 1;
   options.separator = p;
   options.separator_len = strlen (p);
 
@@ -248,7 +162,6 @@ init_sep (variable * v)
 	goto set_default;
     }
 
-  v->bad = 0;
   return;
 
 set_default:
@@ -257,151 +170,67 @@ set_default:
 }
 
 
-static void
-show_sep (variable * v)
-{
-  st_printf ("%s  \"%s\"\n", var_source (v), options.separator);
-}
+static variable variable_table[] = {
 
+  /* Unit number that will be preconnected to standard input */
+  { "GFORTRAN_STDIN_UNIT", GFC_STDIN_UNIT_NUMBER, &options.stdin_unit,
+    init_integer },
 
-static void
-init_string (variable * v __attribute__ ((unused)))
-{
-}
+  /* Unit number that will be preconnected to standard output */
+  { "GFORTRAN_STDOUT_UNIT", GFC_STDOUT_UNIT_NUMBER, &options.stdout_unit,
+    init_integer },
 
-static void
-show_string (variable * v)
-{
-  const char *p;
+  /* Unit number that will be preconnected to standard error */
+  { "GFORTRAN_STDERR_UNIT", GFC_STDERR_UNIT_NUMBER, &options.stderr_unit,
+    init_integer },
 
-  p = getenv (v->name);
-  if (p == NULL)
-    p = "";
+  /* If TRUE, all output will be unbuffered */
+  { "GFORTRAN_UNBUFFERED_ALL", 0, &options.all_unbuffered, init_boolean },
 
-  estr_write (var_source (v));
-  estr_write ("  \"");
-  estr_write (p);
-  estr_write ("\"\n");
-}
+  /* If TRUE, output to preconnected units will be unbuffered */
+  { "GFORTRAN_UNBUFFERED_PRECONNECTED", 0, &options.unbuffered_preconnected,
+    init_boolean },
 
+  /* Whether to print filename and line number on runtime error */
+  { "GFORTRAN_SHOW_LOCUS", 1, &options.locus, init_boolean },
 
-static variable variable_table[] = {
-  {"GFORTRAN_STDIN_UNIT", GFC_STDIN_UNIT_NUMBER, &options.stdin_unit,
-   init_integer, show_integer,
-   "Unit number that will be preconnected to standard input\n"
-   "(No preconnection if negative)", 0},
-
-  {"GFORTRAN_STDOUT_UNIT", GFC_STDOUT_UNIT_NUMBER, &options.stdout_unit,
-   init_integer, show_integer,
-   "Unit number that will be preconnected to standard output\n"
-   "(No preconnection if negative)", 0},
-
-  {"GFORTRAN_STDERR_UNIT", GFC_STDERR_UNIT_NUMBER, &options.stderr_unit,
-   init_integer, show_integer,
-   "Unit number that will be preconnected to standard error\n"
-   "(No preconnection if negative)", 0},
-
-  {"TMPDIR", 0, NULL, init_string, show_string,
-   "Directory for scratch files.", 0},
-
-  {"GFORTRAN_UNBUFFERED_ALL", 0, &options.all_unbuffered, init_boolean,
-   show_boolean,
-   "If TRUE, all output is unbuffered.  This will slow down large writes "
-   "but can be\nuseful for forcing data to be displayed immediately.", 0},
-
-  {"GFORTRAN_UNBUFFERED_PRECONNECTED", 0, &options.unbuffered_preconnected,
-   init_boolean, show_boolean,
-   "If TRUE, output to preconnected units is unbuffered.", 0},
-
-  {"GFORTRAN_SHOW_LOCUS", 1, &options.locus, init_boolean, show_boolean,
-   "If TRUE, print filename and line number where runtime errors happen.", 0},
-
-  {"GFORTRAN_OPTIONAL_PLUS", 0, &options.optional_plus, init_boolean, show_boolean,
-   "Print optional plus signs in numbers where permitted.  Default FALSE.", 0},
-
-  {"GFORTRAN_DEFAULT_RECL", DEFAULT_RECL, &options.default_recl,
-   init_unsigned_integer, show_integer,
-   "Default maximum record length for sequential files.  Most useful for\n"
-   "adjusting line length of preconnected units.  Default "
-   stringize (DEFAULT_RECL), 0},
-
-  {"GFORTRAN_LIST_SEPARATOR", 0, NULL, init_sep, show_sep,
-   "Separator to use when writing list output.  May contain any number of "
-   "spaces\nand at most one comma.  Default is a single space.", 0},
-
-  /* GFORTRAN_CONVERT_UNIT - Set the default data conversion for
-   unformatted I/O.  */
-  {"GFORTRAN_CONVERT_UNIT", 0, 0, init_unformatted, show_string,
-   "Set format for unformatted files", 0},
-
-  {"GFORTRAN_ERROR_BACKTRACE", -1, &options.backtrace,
-    init_boolean, show_boolean,
-    "Print out a backtrace (if possible) on runtime error", -1},
+  /* Print optional plus signs in numbers where permitted */
+  { "GFORTRAN_OPTIONAL_PLUS", 0, &options.optional_plus, init_boolean },
 
-  {NULL, 0, NULL, NULL, NULL, NULL, 0}
-};
+  /* Default maximum record length for sequential files */
+  { "GFORTRAN_DEFAULT_RECL", DEFAULT_RECL, &options.default_recl,
+    init_unsigned_integer },
 
+  /* Separator to use when writing list output */
+  { "GFORTRAN_LIST_SEPARATOR", 0, NULL, init_sep },
 
-/* init_variables()-- Initialize most runtime variables from
- * environment variables. */
+  /* Set the default data conversion for unformatted I/O */
+  { "GFORTRAN_CONVERT_UNIT", 0, 0, init_unformatted },
 
-void
-init_variables (void)
-{
-  variable *v;
+  /* Print out a backtrace if possible on runtime error */
+  { "GFORTRAN_ERROR_BACKTRACE", -1, &options.backtrace, init_boolean },
 
-  for (v = variable_table; v->name; v++)
-    v->init (v);
-}
+  { NULL, 0, NULL, NULL }
+};
 
 
+/* Initialize most runtime variables from
+ * environment variables. */
+
 void
-show_variables (void)
+init_variables (void)
 {
   variable *v;
-  int n;
-
-  /* TODO: print version number.  */
-  estr_write ("GNU Fortran runtime library version "
-	     "UNKNOWN" "\n\n");
-
-  estr_write ("Environment variables:\n");
-  estr_write ("----------------------\n");
 
   for (v = variable_table; v->name; v++)
     {
-      n = estr_write (v->name);
-      print_spaces (25 - n);
-
-      if (v->show == show_integer)
-	estr_write ("Integer ");
-      else if (v->show == show_boolean)
-	estr_write ("Boolean ");
-      else
-	estr_write ("String  ");
-
-      v->show (v);
-      estr_write (v->desc);
-      estr_write ("\n\n");
+      if (v->var)
+	*v->var = v->default_value;
+      v->init (v);
     }
-
-  /* System error codes */
-
-  estr_write ("\nRuntime error codes:");
-  estr_write ("\n--------------------\n");
-
-  for (n = LIBERROR_FIRST + 1; n < LIBERROR_LAST; n++)
-    if (n < 0 || n > 9)
-      st_printf ("%d  %s\n", n, translate_error (n));
-    else
-      st_printf (" %d  %s\n", n, translate_error (n));
-
-  estr_write ("\nCommand line arguments:\n");
-  estr_write ("  --help               Print this list\n");
-
-  exit (0);
 }
 
+
 /* This is the handling of the GFORTRAN_CONVERT_UNITS environment variable.
    It is called from environ.c to parse this variable, and from
    open.c to determine if the user specified a default for an
@@ -509,7 +338,6 @@ match_word (const char *word, int tok)
   else
     res = ILLEGAL;
   return res;
-
 }
 
 /* Match an integer and store its value in unit_num.  This only works
@@ -523,7 +351,6 @@ match_integer (void)
   while (isdigit (*p))
     unit_num = unit_num * 10 + (*p++ - '0');
   return INTEGER;
-
 }
 
 /* This reads the next token from the GFORTRAN_CONVERT_UNITS variable.
Index: libgfortran/runtime/main.c
===================================================================
--- libgfortran/runtime/main.c	(revision 227174)
+++ libgfortran/runtime/main.c	(working copy)
@@ -120,15 +120,6 @@ init (void)
 
   init_compile_options ();
 
-#ifdef DEBUG
-  /* Check for special command lines.  */
-
-  if (argc > 1 && strcmp (argv[1], "--help") == 0)
-    show_variables ();
-
-  /* if (argc > 1 && strcmp(argv[1], "--resume") == 0) resume();  */
-#endif
-
   random_seed_i4 (NULL, NULL, NULL);
 }
 

Re: [libgfortran,patch] Remove never-used debugging code

[Steve Kargl]

On Tue, Aug 25, 2015 at 06:17:13PM +0200, FX wrote:
> Turns out I missed some of the dead code. And I now also fixed comments and some formatting.
> libgfortran/runtime/environ.c is now much more readable than before.
> The patch is still a no-op, in terms of user functionality.
> OK to commit to trunk?
> 

Certainly, the dead code can go.  But,is this changing
the library ABI?

troutmask:fvwm:kargl[764] nm /mnt/sgk/work/6/lib/libgfortran.a | grep show_
0000000000000000 T _gfortrani_show_variables
0000000000000000 t show_boolean
0000000000000000 t show_integer
0000000000000000 t show_sep
0000000000000000 t show_string
0000000000000000 T _gfortrani_show_locus




-- 
Steve

Re: [libgfortran,patch] Remove never-used debugging code

[FX]

> Certainly, the dead code can go.  But,is this changing the library ABI?
> 
> troutmask:fvwm:kargl[764] nm /mnt/sgk/work/6/lib/libgfortran.a | grep show_
> 0000000000000000 T _gfortrani_show_variables
> 0000000000000000 t show_boolean
> 0000000000000000 t show_integer
> 0000000000000000 t show_sep
> 0000000000000000 t show_string
> 0000000000000000 T _gfortrani_show_locus

Nope, none of those functions are actually publicly exported. They are not in gfortran.map, being either static, or having _gfortrani_ prefix which means internal libgfortran use.

FX

Re: [libgfortran,patch] Remove never-used debugging code

[Steve Kargl]

On Tue, Aug 25, 2015 at 07:10:23PM +0200, FX wrote:
> > Certainly, the dead code can go.  But,is this changing the library ABI?
> > 
> > troutmask:fvwm:kargl[764] nm /mnt/sgk/work/6/lib/libgfortran.a | grep show_
> > 0000000000000000 T _gfortrani_show_variables
> > 0000000000000000 t show_boolean
> > 0000000000000000 t show_integer
> > 0000000000000000 t show_sep
> > 0000000000000000 t show_string
> > 0000000000000000 T _gfortrani_show_locus
> 
> Nope, none of those functions are actually publicly exported.
> They are not in gfortran.map, being either static, or having
> _gfortrani_ prefix which means internal libgfortran use.
> 

OK. Just checking.  Thanks for the code cleanup.

-- 
Steve

Re: [libgfortran,patch] Remove never-used debugging code

[FX]

> OK. Just checking.  Thanks for the code cleanup.

Thanks for the review. Committed as rev. 227208.

FX