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

From: John Steele Scott <[email-masked]>
Date: Tue, 13 Nov 2018 09:11:36 +1030

On 13/11/18 8:53 am, Paul Eggert wrote:
> John Steele Scott wrote:
>> 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)
>
> That patch has trouble if 'open' returns 1. The resulting fd is never closed and for some filesystems (NFS, for example) one needs to close the fd to find out whether the fdutimensat call actually worked.

Thanks Paul,

So if someone did want to wipe out that dup2 call from GNU touch, they'd need to track separately whether they opened the file or whether they are using a passed-in file descriptor, rather than relying on the value of fd to indicate that. Got it.

I'll stop flogging this dead horse now. Thanks both Paul and Assaf for your replies.

Cheers,

John