Re: [Nano-devel] [PATCH 2/2] use futimens() if available, instead of utime()

[Kamil Dudka]

Hi Benno,

sorry for not replying earlier.  Your e-mail must have been marked as read 
in my inbox by mistake.  I found it just now while checking the actual state 
of the gnulib patchset.

On Saturday, June 25, 2016 13:53:53 Benno Schulenberg wrote:
> Hello Kamil,
> 
> When Mike commented (about the futimens stuff) that we ought
> to start using gnulib, I thought that this patch would soon
> be made superfluous, so I didn't look at it any further.

Only half of the patch actually.  The #ifdef blocks and autoconf checks will 
be handled by gnulib.  Still the code of nano needs to be changed to use file 
descriptors instead of file names if possible.  I will take care of rebasing 
(and reducing) the patch once the gnulib patch set is pushed.

> But today I was rummaging a bit through Redhat's bugzilla,
> and thus found an earlier version of your patch
> (https://lists.gnu.org/archive/html/nano-devel/2010-11/msg00002.html)
> and noticed that your new version contained something
> 
> that your earlier version didn't:
> > +   /* copy_file() closed the files, make sure 'f' is not used any more. */
> > +   f = NULL;
> 
> I am not at home in the ecosystem of Redhat, but above addition
> seems to be the result of
> https://bugzilla.redhat.com/show_bug.cgi?id=1177155.

As for the above bug, the actual fix was:

http://pkgs.fedoraproject.org/cgit/rpms/nano.git/commit/?id=b07687ce

The line in question was not included in the fix and I am not able to find
it in any released packaging of nano for Fedora.

> Comment #1 in that bug seems to say it has to do with calling futimens()
> on a closed file, but I don't see how that would work.  To me it looks
> like it would be good to set f to NULL there even without any of the
> futimens changes.  Can you shed some light on what bug the above line
> fixes?

IIRC, it does not fix any existing bug.  It was just meant as defensive
coding to prevent possible bugs while changing the code in the future.

Kamil

> Benno