bug#31332: touch unnecessarily calls dup2

[John Steele Scott]

Hi Assif,

Thanks for your reply.

On 30/10/18 1:47 pm, Assaf Gordon wrote:
> tags 31332 notabug
> close 31332
> stop
>
> On 2018-05-01 4:38 a.m., John Steele Scott wrote:
>>  From 
>> https://stackoverflow.com/questions/40446555/why-does-touch-call-the-dup2-syscall
>>
>> address@hidden:/tmp$ touch --version | head -1
>> touch (GNU coreutils) 8.25
>> address@hidden:/tmp$ strace -ttt touch foo 2>&1 | tail -9
>> 1525170579.952032 open("foo", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666) = 3
>> 1525170579.952080 dup2(3, 0)            = 0
>> 1525170579.952119 close(3)              = 0
>> 1525170579.952156 utimensat(0, NULL, NULL, 0) = 0
>> 1525170579.952209 close(0)              = 0
>> 1525170579.952257 close(1)              = 0
>> 1525170579.952294 close(2)              = 0
>> 1525170579.952333 exit_group(0)         = ?
>> 1525170579.952450 +++ exited with 0 +++
>>
>> My analysis from that discussion:
>>
>>      It's a historical artifact.
>>
>>      The open()+dup2() pattern comes from the fd_reopen() function, which is 
>> used
>>      by several of the programs in the coreutils code base.
>>
>
> Not exactly an "artifact" - but an indented operation.
>
I called it a historical artefact because the call to dup2() did not occur when 
touch started calling fd_reopen(). Prior to 
http://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 if stdin was the desired fd. No call to dup2().


> The goal of "fd_reopen" is not just to open a file,
> but to ensure the returned file descriptor (an "int") has a specific
> value.
>
> For example, standard input (STDIN) is typically file descriptor value 0. 
> calling fd_reopen first opens the file (the kernel returns file descriptor 
> 3), then it ensures file descriptor 0 is associated with the same file (and 
> then, calling "close(3)" gets rid of the other file descriptor).
>
> Checking how fd_reopen is used in coreutils, it is almost always used
> to ensure STDIN/STDOUT point to the desired file(s):
> ====
> $ git grep fd_reopen
> src/csplit.c:  if (! STREQ (name, "-") && fd_reopen (STDIN_FILENO, name, 
> O_RDONLY, 0) < 0)
> src/dd.c:/* Restart on EINTR from fd_reopen().  */
> src/dd.c:ifd_reopen (int desired_fd, char const *file, int flag, mode_t mode)
> src/dd.c:      ret = fd_reopen (desired_fd, file, flag, mode);
> src/dd.c:      if (ifd_reopen (STDIN_FILENO, input_file, O_RDONLY | 
> input_flags, 0) < 0)
> src/dd.c:           || ifd_reopen (STDOUT_FILENO, output_file, O_RDWR | opts, 
> perms) < 0)
> src/dd.c:          && (ifd_reopen (STDOUT_FILENO, output_file, O_WRONLY | 
> opts, perms)
> src/nohup.c:      if (fd_reopen (STDIN_FILENO, "/dev/null", O_WRONLY, 0) < 0)
> src/nohup.c:                ? fd_reopen (STDOUT_FILENO, file, flags, mode)
> src/nohup.c:                        ? fd_reopen (STDOUT_FILENO, in_home, 
> flags, mode)
> src/split.c:      && fd_reopen (STDIN_FILENO, infile, O_RDONLY, 0) < 0)
> src/stty.c:      if (fd_reopen (STDIN_FILENO, device_name, O_RDONLY | 
> O_NONBLOCK, 0) < 0)
> src/touch.c:      fd = fd_reopen (STDIN_FILENO, file,
> ===
>
> This means the rest of the program can just operate on STDIN (or STDOUT). 

I haven't looked at all those cases, but my point was that the call to dup2() 
adds an extra 40us to each invocation of touch. In the case of touch, it really 
only cares about the fd being STDOUT_FILENO or !STDOUT_FILENO; it doesn't need 
to be STDIN_FILENO exactly. With a small amount of work touch could use open() 
instead of fd_reopen(). The code would be somewhat simpler and it would save 
40us of execution time.

Or maybe it wouldn't, since there's an extra fd open when the program 
terminates and we need to wait for the kernel to clean that up anyway? I don't 
know.

Cheers,

John