[SV 62840] Don't change IO buffering before printing version

If users run 'make --version | head -n1' they expect make to exit
with a success (0) code.  This works because the pipe forces the
default buffering on stdout to be fully buffered so all the output
is printed to the pipe in a single write(2) and won't fail.  However
due to output sync we forcibly set stdout to line buffered, which
means if the reader closes the pipe fast enough make will exit with
an error code because the write to stdout failed.

Move the setup done in output_init() back into main() where it can
be done in a proper order.  Rework the order of operations during
startup so that we check for help and version flags before we change
the buffering.  Clean up the behavior of print_usage().

Original changes from Dmitry Goncharov <dgoncharov@users.sf.net>.

* src/main.c (switches): Don't send --version in the environment.
(print_usage): Add a blank line after the version before the usage.
Move the die() into this function since we always die() afterward.
Note the die will flush so no need to do it explicitly.
(print_version): The caller will fflush when appropriate.
(close_stdout): Move from output.c so it is installed early.
(decode_switches): Only call print_usage on error, not for --help.
(main): Install the close_stdout handler immediately after start.
Move the calls to print_usage() due to --help and --version to be
called immediately after we decode the switches.  Move the buffer set
here from output_init(), immediately after we know we'll be running.
* src/output.c (output_init): Move buffer setting to main().
(close_stdout): Move to main().

diff --git a/src/main.c b/src/main.c
index 799b2e912b53ee20ffe2b0f073e1efe7833865a7..1deb926df74ea0ee600f721ea47a72948927b10c 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -460,7 +460,7 @@ static const struct command_switch switches[] =
     { 'S', flag_off, &keep_going_flag, 1, 1, 0, 0, &default_keep_going_flag,
       "no-keep-going" },
     { 't', flag, &touch_flag, 1, 1, 1, 0, 0, "touch" },
-    { 'v', flag, &print_version_flag, 1, 1, 0, 0, 0, "version" },
+    { 'v', flag, &print_version_flag, 1, 0, 0, 0, 0, "version" },
     { 'w', flag, &print_directory_flag, 1, 1, 0, 0,
       &default_print_directory_flag, "print-directory" },
 
@@ -686,6 +686,52 @@ initialize_stopchar_map (void)
     }
 }
 
+/* This code is stolen from gnulib.
+   If/when we abandon the requirement to work with K&R compilers, we can
+   remove this (and perhaps other parts of GNU make!) and migrate to using
+   gnulib directly.
+
+   This is called only through atexit(), which means die() has already been
+   invoked.  So, call exit() here directly.  Apparently that works...?
+*/
+
+/* Close standard output, exiting with status 'exit_failure' on failure.
+   If a program writes *anything* to stdout, that program should close
+   stdout and make sure that it succeeds before exiting.  Otherwise,
+   suppose that you go to the extreme of checking the return status
+   of every function that does an explicit write to stdout.  The last
+   printf can succeed in writing to the internal stream buffer, and yet
+   the fclose(stdout) could still fail (due e.g., to a disk full error)
+   when it tries to write out that buffered data.  Thus, you would be
+   left with an incomplete output file and the offending program would
+   exit successfully.  Even calling fflush is not always sufficient,
+   since some file systems (NFS and CODA) buffer written/flushed data
+   until an actual close call.
+
+   Besides, it's wasteful to check the return value from every call
+   that writes to stdout -- just let the internal stream state record
+   the failure.  That's what the ferror test is checking below.
+
+   It's important to detect such failures and exit nonzero because many
+   tools (most notably 'make' and other build-management systems) depend
+   on being able to detect failure in other tools via their exit status.  */
+
+static void
+close_stdout (void)
+{
+  int prev_fail = ferror (stdout);
+  int fclose_fail = fclose (stdout);
+
+  if (prev_fail || fclose_fail)
+    {
+      if (fclose_fail)
+        perror_with_name (_("write error: stdout"), "");
+      else
+        O (error, NILF, _("write error: stdout"));
+      exit (MAKE_TROUBLE);
+    }
+}
+
 static const char *
 expand_command_line_file (const char *name)
 {
@@ -833,6 +879,38 @@ decode_output_sync_flags (void)
 #endif
 }
 
+/* Print a nice usage method and exit.  */
+
+static void NORETURN
+print_usage (int bad)
+{
+  const char *const *cpp;
+  FILE *usageto;
+
+  if (print_version_flag)
+    {
+      print_version ();
+      fputs ("\n", stdout);
+    }
+
+  usageto = bad ? stderr : stdout;
+
+  fprintf (usageto, _("Usage: %s [options] [target] ...\n"), program);
+
+  for (cpp = usage; *cpp; ++cpp)
+    fputs (_(*cpp), usageto);
+
+  if (!remote_description || *remote_description == '\0')
+    fprintf (usageto, _("\nThis program built for %s\n"), make_host);
+  else
+    fprintf (usageto, _("\nThis program built for %s (%s)\n"),
+             make_host, remote_description);
+
+  fprintf (usageto, _("Report bugs to <bug-make@gnu.org>\n"));
+
+  die (bad ? MAKE_FAILURE : MAKE_SUCCESS);
+}
+
 #ifdef WINDOWS32
 
 /*
@@ -1089,6 +1167,11 @@ main (int argc, char **argv, char **envp)
   /* Useful for attaching debuggers, etc.  */
   SPIN ("main-entry");
 
+#ifdef HAVE_ATEXIT
+  if (ANY_SET (check_io_state (), IO_STDOUT_OK))
+    atexit (close_stdout);
+#endif
+
   output_init (&make_sync);
 
   initialize_stopchar_map();
@@ -1501,6 +1584,23 @@ main (int argc, char **argv, char **envp)
       arg_job_slots = env_slots;
   }
 
+  if (print_usage_flag)
+    print_usage (0);
+
+  /* Print version information, and exit.  */
+  if (print_version_flag)
+    {
+      print_version ();
+      die (MAKE_SUCCESS);
+    }
+
+  /* Now that we know we'll be running, force stdout to be line-buffered.  */
+#ifdef HAVE_SETVBUF
+  setvbuf (stdout, 0, _IOLBF, BUFSIZ);
+#elif HAVE_SETLINEBUF
+  setlinebuf (stdout);
+#endif
+
   /* Handle shuffle mode argument.  */
   if (shuffle_mode)
     {
@@ -1569,15 +1669,14 @@ main (int argc, char **argv, char **envp)
   if (no_builtin_variables_flag)
     no_builtin_rules_flag = 1;
 
-  /* Print version information, and exit.  */
-  if (print_version_flag)
+  if (ISDB (DB_BASIC))
     {
       print_version ();
-      die (MAKE_SUCCESS);
-    }
 
-  if (ISDB (DB_BASIC))
-    print_version ();
+      /* Flush stdout so the user doesn't have to wait to see the
+         version information while make thinks about things.  */
+      fflush (stdout);
+    }
 
 #ifndef VMS
   /* Set the "MAKE_COMMAND" variable to the name we were invoked with.
@@ -2960,33 +3059,6 @@ handle_non_switch_argument (const char *arg, int env)
     }
 }
 
-/* Print a nice usage method.  */
-
-static void
-print_usage (int bad)
-{
-  const char *const *cpp;
-  FILE *usageto;
-
-  if (print_version_flag)
-    print_version ();
-
-  usageto = bad ? stderr : stdout;
-
-  fprintf (usageto, _("Usage: %s [options] [target] ...\n"), program);
-
-  for (cpp = usage; *cpp; ++cpp)
-    fputs (_(*cpp), usageto);
-
-  if (!remote_description || *remote_description == '\0')
-    fprintf (usageto, _("\nThis program built for %s\n"), make_host);
-  else
-    fprintf (usageto, _("\nThis program built for %s (%s)\n"),
-             make_host, remote_description);
-
-  fprintf (usageto, _("Report bugs to <bug-make@gnu.org>\n"));
-}
-
 /* Decode switches from ARGC and ARGV.
    They came from the environment if ENV is nonzero.  */
 
@@ -3187,11 +3259,8 @@ decode_switches (int argc, const char **argv, int env)
   while (optind < argc)
     handle_non_switch_argument (argv[optind++], env);
 
-  if (!env && (bad || print_usage_flag))
-    {
-      print_usage (bad);
-      die (bad ? MAKE_FAILURE : MAKE_SUCCESS);
-    }
+  if (bad && !env)
+    print_usage (bad);
 
   /* If there are any options that need to be decoded do it now.  */
   decode_debug_flags ();
@@ -3558,10 +3627,6 @@ print_version (void)
             precede, precede, precede);
 
   printed_version = 1;
-
-  /* Flush stdout so the user doesn't have to wait to see the
-     version information while make thinks about things.  */
-  fflush (stdout);
 }
 
 /* Print a bunch of information about this and that.  */

diff --git a/src/output.c b/src/output.c
index 91a1dc31b3594168fdee8ca1cb54d99066242bef..d806159f1d2f94ff73a1dc2043f3ff78b948a9b8 100644 (file)
--- a/src/output.c
+++ b/src/output.c
@@ -299,53 +299,6 @@ output_dump (struct output *out)
 #endif /* NO_OUTPUT_SYNC */
 \f
 
-/* This code is stolen from gnulib.
-   If/when we abandon the requirement to work with K&R compilers, we can
-   remove this (and perhaps other parts of GNU make!) and migrate to using
-   gnulib directly.
-
-   This is called only through atexit(), which means die() has already been
-   invoked.  So, call exit() here directly.  Apparently that works...?
-*/
-
-/* Close standard output, exiting with status 'exit_failure' on failure.
-   If a program writes *anything* to stdout, that program should close
-   stdout and make sure that it succeeds before exiting.  Otherwise,
-   suppose that you go to the extreme of checking the return status
-   of every function that does an explicit write to stdout.  The last
-   printf can succeed in writing to the internal stream buffer, and yet
-   the fclose(stdout) could still fail (due e.g., to a disk full error)
-   when it tries to write out that buffered data.  Thus, you would be
-   left with an incomplete output file and the offending program would
-   exit successfully.  Even calling fflush is not always sufficient,
-   since some file systems (NFS and CODA) buffer written/flushed data
-   until an actual close call.
-
-   Besides, it's wasteful to check the return value from every call
-   that writes to stdout -- just let the internal stream state record
-   the failure.  That's what the ferror test is checking below.
-
-   It's important to detect such failures and exit nonzero because many
-   tools (most notably 'make' and other build-management systems) depend
-   on being able to detect failure in other tools via their exit status.  */
-
-static void
-close_stdout (void)
-{
-  int prev_fail = ferror (stdout);
-  int fclose_fail = fclose (stdout);
-
-  if (prev_fail || fclose_fail)
-    {
-      if (fclose_fail)
-        perror_with_name (_("write error: stdout"), "");
-      else
-        O (error, NILF, _("write error: stdout"));
-      exit (MAKE_TROUBLE);
-    }
-}
-\f
-
 void
 output_init (struct output *out)
 {
@@ -356,23 +309,10 @@ output_init (struct output *out)
       return;
     }
 
-  /* Configure this instance of make.  Be sure stdout is line-buffered.  */
-
-#ifdef HAVE_SETVBUF
-  setvbuf (stdout, 0, _IOLBF, BUFSIZ);
-#elif HAVE_SETLINEBUF
-  setlinebuf (stdout);
-#endif  /* setlinebuf missing.  */
-
-  /* Force stdout/stderr into append mode.  This ensures parallel jobs won't
-     lose output due to overlapping writes.  */
+  /* Force stdout/stderr into append mode (if they are files) to ensure
+     parallel jobs won't lose output due to overlapping writes.  */
   fd_set_append (fileno (stdout));
   fd_set_append (fileno (stderr));
-
-#ifdef HAVE_ATEXIT
-  if (ANY_SET (check_io_state (), IO_STDOUT_OK))
-    atexit (close_stdout);
-#endif
 }
 
 void