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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11816 in the body.
You can then email your comments to 11816 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: bug-coreutils <at> gnu.org
Subject: sort -o: error comes late if opening the outfile fails
Date: Fri, 29 Jun 2012 13:54:32 +0200

If opening the output file for writing is not possible - e.g. because the user
doesn't have sufficient privileges, then sort issues an error. The problem
is that the whole - then useless - computation is already done.

In the following little example, it took ~15s to sort the data,
and then to realize that it can't write the result:

  $ time seq 1000000 | src/sort --random-sort -o /cantwritehere
  src/sort: open failed: /cantwritehere: Permission denied

  real    0m14.955s
  user    0m14.936s
  sys     0m0.042s

I'd have expected sort to give the error immediately after startup
instead of wasting time for sorting.
Shouldn't sort open the outfile right at the beginning (unless in==out),
or is this behavior required by some standard?

Have a nice day,
Berny

Message #8 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>
Cc: 11816 <at> debbugs.gnu.org
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Fri, 29 Jun 2012 13:55:59 +0100

On 06/29/2012 12:54 PM, Bernhard Voelker wrote:
> If opening the output file for writing is not possible - e.g. because the user
> doesn't have sufficient privileges, then sort issues an error. The problem
> is that the whole - then useless - computation is already done.
> 
> In the following little example, it took ~15s to sort the data,
> and then to realize that it can't write the result:
> 
>   $ time seq 1000000 | src/sort --random-sort -o /cantwritehere
>   src/sort: open failed: /cantwritehere: Permission denied
> 
>   real    0m14.955s
>   user    0m14.936s
>   sys     0m0.042s
> 
> I'd have expected sort to give the error immediately after startup
> instead of wasting time for sorting.
> Shouldn't sort open the outfile right at the beginning (unless in==out),
> or is this behavior required by some standard?

Hmm I think that would be an improvement.
Now if there was an issue with sorting (like running out of resources),
we'd prefer not to leave the empty output hanging around.
Also the in==out case, you'd like to check for write-ability too.

Both cases could be handled I think with something like:

if (access (outfile, W_OK) != 0 && errno != ENOENT)
  error (...);

cheers,
Pádraig.

Message #11 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 11816 <at> debbugs.gnu.org, Bernhard Voelker <mail <at> bernhard-voelker.de>
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Sat, 30 Jun 2012 06:53:06 -0500

On 06/29/2012 07:55 AM, Pádraig Brady wrote:
> Also the in==out case, you'd like to check for write-ability too.
>
> Both cases could be handled I think with something like:
>
> if (access (outfile, W_OK) != 0 && errno != ENOENT)
>   error (...);

Wouldn't it be better to actually open the output file,
but not truncate it?  We can then truncate it just before
actually writing to the file.  That would avoid a race
condition or two.

In the in==out case, we could tune this by opening
the file just once, with O_RDWR.  If the file is not
a regular file, we might have to give up and open such
a file twice, but that should be rare.

Message #14 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11816 <at> debbugs.gnu.org, Bernhard Voelker <mail <at> bernhard-voelker.de>
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Sat, 30 Jun 2012 14:11:08 +0100

On 06/30/2012 12:53 PM, Paul Eggert wrote:
> On 06/29/2012 07:55 AM, Pádraig Brady wrote:
>> Also the in==out case, you'd like to check for write-ability too.
>>
>> Both cases could be handled I think with something like:
>>
>> if (access (outfile, W_OK) != 0 && errno != ENOENT)
>>   error (...);
> 
> Wouldn't it be better to actually open the output file,
> but not truncate it?  We can then truncate it just before
> actually writing to the file.  That would avoid a race
> condition or two.
> 
> In the in==out case, we could tune this by opening
> the file just once, with O_RDWR.  If the file is not
> a regular file, we might have to give up and open such
> a file twice, but that should be rare.
> 

The race would be unlikely and
only fallback to the existing operation
of slower failure.

Though I suppose opening the file is a
more direct check and would also obviate the
need to check for writeability of the containing dir
in the case of a non existent file.

OK I'm leaning towards an early open so.

As for cleaning up an empty created file,
`sort` already has an exit_cleanup() function,
so we can unlink there.

I'm not sure it's worth tuning the in==out case TBH.

cheers,
Pádraig.

Message #19 received at 11816-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11816-done <at> debbugs.gnu.org, Bernhard Voelker <mail <at> bernhard-voelker.de>
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Mon, 02 Jul 2012 12:21:38 +0200

[Message part 1 (text/plain, inline)]

On 06/30/2012 03:11 PM, Pádraig Brady wrote:
> On 06/30/2012 12:53 PM, Paul Eggert wrote:
>> On 06/29/2012 07:55 AM, Pádraig Brady wrote:
>>> Also the in==out case, you'd like to check for write-ability too.
>>>
>>> Both cases could be handled I think with something like:
>>>
>>> if (access (outfile, W_OK) != 0 && errno != ENOENT)
>>>   error (...);
>>
>> Wouldn't it be better to actually open the output file,
>> but not truncate it?  We can then truncate it just before
>> actually writing to the file.  That would avoid a race
>> condition or two.
>>
>> In the in==out case, we could tune this by opening
>> the file just once, with O_RDWR.  If the file is not
>> a regular file, we might have to give up and open such
>> a file twice, but that should be rare.
>>
> 
> The race would be unlikely and
> only fallback to the existing operation
> of slower failure.
> 
> Though I suppose opening the file is a
> more direct check and would also obviate the
> need to check for writeability of the containing dir
> in the case of a non existent file.
> 
> OK I'm leaning towards an early open so.
> 
> As for cleaning up an empty created file,
> `sort` already has an exit_cleanup() function,
> so we can unlink there.
> 
> I'm not sure it's worth tuning the in==out case TBH.

So I didn't bother unlinking created empty files
as this is problematic in the presence of symlinks.
To mitigate this I create the output after all option
validation is done, just before sort/merge process is started.

Also we must be careful to handle the `sort -o missing missing` case.
I.E. we don't want to create an empty file, resulting in the
above failing to notice the missing file and returning succesfully.
So to avoid that I explicitly check all inputs are readable first.
In addition to catering for the above case, it's a general improvement
to avoid redundant processing.  That was already handled in the merge case,
but in the sorting case only a stat was done as a side effect
of input size checking, and that didn't handle the case
where input was present but unreadable.

Patch attached.

cheers,
Pádraig.

[sort-exit-early.diff (text/plain, attachment)]

Message #22 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 11816 <at> debbugs.gnu.org
Cc: P <at> draigBrady.com
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Mon, 02 Jul 2012 13:26:46 +0200

Pádraig Brady wrote:
...
> Subject: [PATCH] sort: avoid redundant processing with inaccessible inputs or
>  output
>
> * src/sort.c (check_inputs): A new function to verify all inputs
> are accessible before further processing.
> (check_output): A new function to open or create a specified
> output file, before futher processing.
> (stream_open): Adjust to truncating the previously opened
> output file rather than opening directly.
> (avoid_trashing_input): Optimize to stat the output file
> descriptor, rather than the file name.
> (main): Call the new functions to check accessability of

Hi Pádraig,

Thanks for dealing with this.
So far, I've read through the commit log and NEWS:

s/accessability/accessibility/

> inputs and output, before processing starts.
...
> diff --git a/NEWS b/NEWS
> index 8c75a32..c51fb78 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -41,6 +41,12 @@ GNU coreutils NEWS                                    -*- outline -*-
>    patches as well as enough support to build on the Hurd, we no longer
>    have any reason to include it here.
>
> +** Improvements
> +
> +  sort will avoid redundant processing in the presence of inaccessible inputs,

s/will avoid/avoids/ (or "now avoids") ?

> +  or unwritable output.  Immediate errors are now given before any potentially
> +  expensive processing is initiated.

Maybe replace that "Immediate ..." sentence with something like this:

  Sort now diagnoses certain errors at start-up, rather than
  only after potentially expensive processing.

Message #25 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: 11816 <at> debbugs.gnu.org
Cc: P <at> draigBrady.com
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Mon, 02 Jul 2012 18:55:32 +0200

Pádraig Brady wrote:
>...
> diff --git a/tests/misc/sort-exit-early b/tests/misc/sort-exit-early
...
> +test $? == 124 && fail=1
> +
> +# Check all inputs are readable before starting to sort
> +# Also ensure the output isn't created in this case
> +touch output
> +chmod a-r output
> +timeout 10 sort -o typo - output && fail=1
> +test $? == 124 && fail=1

"make syntax-check" dinged these:

  tests/misc/sort-exit-early:24:test $? == 124 && fail=1
  tests/misc/sort-exit-early:31:test $? == 124 && fail=1
  maint.mk: use "test x = x", not "test x == x"
  make: *** [sc_prohibit_test_double_equal] Error 1


Here's a fix for a formatting nit in the sources:

   static void
  -check_inputs (char * const *files, size_t nfiles)
  +check_inputs (char *const *files, size_t nfiles)

Otherwise, it all looks fine.
Thanks again.

Message #28 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 11816 <at> debbugs.gnu.org
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Mon, 02 Jul 2012 21:24:48 +0200

On 07/02/2012 06:55 PM, Jim Meyering wrote:
> Pádraig Brady wrote:
>> ...
>> diff --git a/tests/misc/sort-exit-early b/tests/misc/sort-exit-early
> ...
>> +test $? == 124 && fail=1
>> +
>> +# Check all inputs are readable before starting to sort
>> +# Also ensure the output isn't created in this case
>> +touch output
>> +chmod a-r output
>> +timeout 10 sort -o typo - output && fail=1
>> +test $? == 124 && fail=1
> 
> "make syntax-check" dinged these:
> 
>   tests/misc/sort-exit-early:24:test $? == 124 && fail=1
>   tests/misc/sort-exit-early:31:test $? == 124 && fail=1
>   maint.mk: use "test x = x", not "test x == x"
>   make: *** [sc_prohibit_test_double_equal] Error 1
> 
> 
> Here's a fix for a formatting nit in the sources:
> 
>    static void
>   -check_inputs (char * const *files, size_t nfiles)
>   +check_inputs (char *const *files, size_t nfiles)
> 
> Otherwise, it all looks fine.
> Thanks again.

Pushed.
thanks for the careful review.

Pádraig.

Message #31 received at 11816 <at> debbugs.gnu.org (full text, mbox):

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: Re: 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

Message #34 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11816 <at> debbugs.gnu.org, Jim Meyering <jim <at> meyering.net>
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Tue, 03 Jul 2012 01:08:04 +0200

On 07/03/2012 12:49 AM, Paul Eggert wrote:
> 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));

Hmm I know I had EXIT_FAILURE here.
Should that be SORT_FAILURE?

> +      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);
>      }
>  }
>  

Looks good!

cheers,
Pádraig.

Message #37 received at 11816 <at> debbugs.gnu.org (full text, mbox):

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: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Mon, 02 Jul 2012 16:49:41 -0700

On 07/02/2012 04:08 PM, Pádraig Brady wrote:
> Hmm I know I had EXIT_FAILURE here.
> Should that be SORT_FAILURE?

Yes, thanks, good catch.  I pushed that as
a further fix.

Message #40 received at 11816 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11816 <at> debbugs.gnu.org, Pádraig Brady <P <at> draigBrady.com>
Subject: Re: bug#11816: sort -o: error comes late if opening the outfile fails
Date: Tue, 03 Jul 2012 10:25:59 +0200

Paul Eggert wrote:
> 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.

Nice further improvement.
Thanks!

Previous Next

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