GNU bug report logs - #11816
sort -o: error comes late if opening the outfile fails

Previous Next

Full log

View this message in rfc822 format

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 11816 <at> debbugs.gnu.org, Jim Meyering <jim <at> meyering.net>
Subject: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Mon, 02 Jul 2012 15:49:45 -0700

Thanks for the improvement.
How about the following patch to simplify this a bit?
It removes a call to fdopen, among other things.

From 05cc1b416a47330ef296dbeadd2a4b6095fe5c7d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Mon, 2 Jul 2012 15:47:03 -0700
Subject: [PATCH] sort: simplify -o handling to avoid fdopen, assert

* src/sort.c (outfd): Remove.  All uses replaced by STDOUT_FILENO.
(stream_open): When writing, use stdout rather than fdopen.
(move_fd_or_die): Renamed from dup2_or_die, with the added functionality
of closing its first argument.  All uses changed.
(avoid_trashing_input): Special case for !outfile no longer needed.
(check_output): Arrange for standard output to go to the file,
rather than storing the fd in outfd.
---
 src/sort.c |   47 ++++++++++++++++++++---------------------------
 1 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/src/sort.c b/src/sort.c
index e4067c9..f0d32c3 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -357,9 +357,6 @@ static bool unique;
 /* Nonzero if any of the input files are the standard input. */
 static bool have_read_stdin;
 
-/* File descriptor associated with -o.  */
-static int outfd = -1;
-
 /* List of key field comparisons to be tried.  */
 static struct keyfield *keylist;
 
@@ -916,8 +913,6 @@ stream_open (char const *file, char const *how)
 {
   FILE *fp;
 
-  if (!file)
-    return stdout;
   if (*how == 'r')
     {
       if (STREQ (file, "-"))
@@ -931,10 +926,10 @@ stream_open (char const *file, char const *how)
     }
   else if (*how == 'w')
     {
-      assert (outfd != -1);
-      if (ftruncate (outfd, 0) != 0)
-        error (EXIT_FAILURE, errno, _("%s: error truncating"), quote (file));
-      fp = fdopen (outfd, how);
+      if (file && ftruncate (STDOUT_FILENO, 0) != 0)
+        error (EXIT_FAILURE, errno, _("%s: error truncating"),
+               quote (file));
+      fp = stdout;
     }
   else
     assert (!"unexpected mode passed to stream_open");
@@ -981,10 +976,14 @@ xfclose (FILE *fp, char const *file)
 }
 
 static void
-dup2_or_die (int oldfd, int newfd)
+move_fd_or_die (int oldfd, int newfd)
 {
-  if (dup2 (oldfd, newfd) < 0)
-    error (SORT_FAILURE, errno, _("dup2 failed"));
+  if (oldfd != newfd)
+    {
+      if (dup2 (oldfd, newfd) < 0)
+        error (SORT_FAILURE, errno, _("dup2 failed"));
+      close (oldfd);
+    }
 }
 
 /* Fork a child process for piping to and do common cleanup.  The
@@ -1095,10 +1094,8 @@ maybe_create_temp (FILE **pfp, bool survive_fd_exhaustion)
       else if (node->pid == 0)
         {
           close (pipefds[1]);
-          dup2_or_die (tempfd, STDOUT_FILENO);
-          close (tempfd);
-          dup2_or_die (pipefds[0], STDIN_FILENO);
-          close (pipefds[0]);
+          move_fd_or_die (tempfd, STDOUT_FILENO);
+          move_fd_or_die (pipefds[0], STDIN_FILENO);
 
           if (execlp (compress_program, compress_program, (char *) NULL) < 0)
             error (SORT_FAILURE, errno, _("couldn't execute %s"),
@@ -1155,10 +1152,8 @@ open_temp (struct tempnode *temp)
 
     case 0:
       close (pipefds[0]);
-      dup2_or_die (tempfd, STDIN_FILENO);
-      close (tempfd);
-      dup2_or_die (pipefds[1], STDOUT_FILENO);
-      close (pipefds[1]);
+      move_fd_or_die (tempfd, STDIN_FILENO);
+      move_fd_or_die (pipefds[1], STDOUT_FILENO);
 
       execlp (compress_program, compress_program, "-d", (char *) NULL);
       error (SORT_FAILURE, errno, _("couldn't execute %s -d"),
@@ -3649,10 +3644,7 @@ avoid_trashing_input (struct sortfile *files, size_t ntemps,
         {
           if (! got_outstat)
             {
-              if ((outfile
-                   ? fstat (outfd, &outstat)
-                   : fstat (STDOUT_FILENO, &outstat))
-                  != 0)
+              if (fstat (STDOUT_FILENO, &outstat) != 0)
                 break;
               got_outstat = true;
             }
@@ -3703,17 +3695,18 @@ check_inputs (char *const *files, size_t nfiles)
 }
 
 /* Ensure a specified output file can be created or written to,
-   and cache the returned descriptor in the global OUTFD variable.
-   Otherwise exit with a diagnostic.  */
+   and point stdout to it.  Do not truncate the file.
+   Exit with a diagnostic on failure.  */
 
 static void
 check_output (char const *outfile)
 {
   if (outfile)
     {
-      outfd = open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE_RW_UGO);
+      int outfd = open (outfile, O_WRONLY | O_CREAT | O_BINARY, MODE_RW_UGO);
       if (outfd < 0)
         die (_("open failed"), outfile);
+      move_fd_or_die (outfd, STDOUT_FILENO);
     }
 }
 
-- 
1.7.6.5

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.