Subject: Re: bug#31332: touch unnecessarily calls dup2

From: John Steele Scott <[email-masked]>
Date: Tue, 30 Oct 2018 16:44:45 +1030

On 30/10/18 4:08 pm, Paul Eggert wrote:
> John Steele Scott wrote:
>> Prior tohttp://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=e373bb19  fd_reopen() initially did "close(desired_fd); fd = open(...)" which would always do the right thing
>
> No, as the old code did the wrong thing for "touch /dev/stdin", whereas the current code works.

I stand corrected. Okay, I see that we don't want to close /dev/stdin at the start of touch. Thanks for making me think of this case.

I'd much much obliged if one of you could enlighten me as to why touch needs to treat /dev/stdin as a special case after the file has been opened though. Wouldn't the following work just as well, with one less system call?

--- a/src/touch.c
+++ b/src/touch.c
@@ -132,8 +132,8 @@ touch (const char *file)
   else if (! (no_create || no_dereference))
     {
       /* Try to open FILE, creating it if necessary.  */
-      fd = fd_reopen (STDIN_FILENO, file,
-                      O_WRONLY | O_CREAT | O_NONBLOCK | O_NOCTTY, MODE_RW_UGO);
+      fd = open (file,
+                 O_WRONLY | O_CREAT | O_NONBLOCK | O_NOCTTY, MODE_RW_UGO);
 
       /* Don't save a copy of errno if it's EISDIR, since that would lead
          touch to give a bogus diagnostic for e.g., 'touch /' (assuming
@@ -166,9 +166,9 @@ touch (const char *file)
                      (no_dereference && fd == -1) ? AT_SYMLINK_NOFOLLOW : 0)
         == 0);
 
-  if (fd == STDIN_FILENO)
+  if (fd != STDOUT_FILENO)
     {
-      if (close (STDIN_FILENO) != 0)
+      if (close (fd) != 0)
         {
           error (0, errno, _("failed to close %s"), quoteaf (file));
           return false;

Cheers,

John