[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Nano-devel] [PATCH v2] pull in the futimens module from gnulib
From: |
Benno Schulenberg |
Subject: |
Re: [Nano-devel] [PATCH v2] pull in the futimens module from gnulib |
Date: |
Mon, 03 Apr 2017 20:02:44 +0200 |
On Mon, Apr 3, 2017, at 10:13, Kamil Dudka wrote:
> /* Read from inn, write to out. We assume inn is opened for reading,
> * and out for writing. We return 0 on success, -1 on read error, or -2
> - * on write error. */
> -int copy_file(FILE *inn, FILE *out)
> + * on write error. inn is always closed by this function, out is closed
> + * only if close_out is true. */
> +int copy_file(FILE *inn, FILE *out, bool close_out)
> {
> int retval = 0;
> char buf[BUFSIZ];
> size_t charsread;
> + int (*flush_out_fnc)(FILE *) = (close_out) ? fclose : fflush;
>
> assert(inn != NULL && out != NULL && inn != out);
>
> @@ -1564,7 +1566,7 @@ int copy_file(FILE *inn, FILE *out)
>
> if (fclose(inn) == EOF)
> retval = -1;
> - if (fclose(out) == EOF)
> + if (flush_out_fnc(out) == EOF)
> retval = -2;
Why is the flush needed? Could the timestamp get changed if
the actual writing happened a few seconds later?
> - struct utimbuf filetime;
> + static struct timespec filetime[2];
Why does it need to be static?
> - filetime.actime = openfile->current_stat->st_atime;
> - filetime.modtime = openfile->current_stat->st_mtime;
> + filetime[/* atime */ 0].tv_sec = openfile->current_stat->st_atime;
> + filetime[/* mtime */ 1].tv_sec = openfile->current_stat->st_mtime;
I don't think the comments are needed; it's obvious enough by
what gets assigned to them.
> /* Copy the file. */
> - copy_status = copy_file(f, backup_file);
> + copy_status = copy_file(f, backup_file, /* close_out */ false);
Here the inline comment is confusing, because nano doesn't
use them anywhere else. Boolean values are always uppercase.
Benno
--
http://www.fastmail.com - Or how I learned to stop worrying and
love email again