Jim Meyering wrote:
The first one is a clear bug fix.
Oops, my bad. Thanks for catching that. I installed a simpler fix (first
attached patch).
For the second, since --no-time (-T) never worked (-m does),
I could also just remove its entry from longopts. We've
done without it for so long, there's little point to adding
an undocumented --no-time, now.
Thanks, I like that idea and installed the 2nd attached patch.
In rereading the code I noticed other problems likely to bite after the year
2038 (3rd attached patch, also installed). This stuff is a pain, as it won't see
realistic testing for another 20 years or so and I don't see easy test cases for
it, partly because GNU/Linux seems to mishandles these time stamps now on my
platform (Fedora 24 x86-64; I'll try to file a bug report about this to Fedora).
Boldly marking the bug as done.
From 0c6cff7709c47c5f8f113bebcd9dc3cf0b533a69 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 4 Nov 2016 20:15:42 -0700
Subject: [PATCH 1/3] gzip --no-name: avoid spurious warning
Problem reported by Jim Meyering (Bug#24826).
* tests/timestamp: Add a test from Jim Meyering to exercise the fix
* zip.c (zip): Treat unknown time stamps as 0.
---
tests/timestamp | 3 +++
zip.c | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/timestamp b/tests/timestamp
index 7acfe5d..141c1d4 100755
--- a/tests/timestamp
+++ b/tests/timestamp
@@ -49,4 +49,7 @@ touch -t 210602070628.15 in || {
test $? = 2 || fail=1
}
+# Ensure that --no-name does not provoke a time stamp warning.
+: | gzip --no-name > k || fail=1
+
Exit $fail
diff --git a/zip.c b/zip.c
index eb60409..a3b4559 100644
--- a/zip.c
+++ b/zip.c
@@ -54,7 +54,9 @@ int zip(in, out)
flags |= ORIG_NAME;
}
put_byte(flags); /* general flags */
- if (0 < time_stamp.tv_sec && time_stamp.tv_sec <= 0xffffffff)
+ if (time_stamp.tv_nsec < 0)
+ stamp = 0;
+ else if (0 < time_stamp.tv_sec && time_stamp.tv_sec <= 0xffffffff)
stamp = time_stamp.tv_sec;
else
{
--
2.7.4
From 118f9562d367d1fd2a856ce4147cff0679f1e2a1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 4 Nov 2016 20:15:42 -0700
Subject: [PATCH 2/3] gzip: --no-time cleanup
Problem reported by Jim Meyering (Bug#24826).
* gzip.c (longopts): Remove non-working no-time entry.
(help) [UNDOCUMENTED]: Don't document it.
---
gzip.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/gzip.c b/gzip.c
index 0fca5a3..dccdb89 100644
--- a/gzip.c
+++ b/gzip.c
@@ -285,7 +285,6 @@ static const struct option longopts[] =
{"recursive", 0, 0, 'r'}, /* recurse through directories */
{"suffix", 1, 0, 'S'}, /* use given suffix instead of .gz */
{"test", 0, 0, 't'}, /* test compressed file integrity */
- {"no-time", 0, 0, 'T'}, /* don't save or restore the time stamp */
{"verbose", 0, 0, 'v'}, /* verbose mode */
{"version", 0, 0, 'V'}, /* display version number */
{"fast", 0, 0, '1'}, /* compress faster */
@@ -358,7 +357,7 @@ local void help()
" -l, --list list compressed file contents",
" -L, --license display software license",
#ifdef UNDOCUMENTED
- " -m, --no-time do not save or restore the original modification time",
+ " -m do not save or restore the original modification time",
" -M, --time save or restore the original modification time",
#endif
" -n, --no-name do not save or restore the original name and time stamp",
@@ -1462,7 +1461,7 @@ discard_input_bytes (nbytes, flags)
* original name was given and to_stdout is not set.
* Return the compression method, -1 for error, -2 for warning.
* Set inptr to the offset of the next byte to be processed.
- * Updates time_stamp if there is one and --no-time is not used.
+ * Updates time_stamp if there is one and neither -m nor -n is used.
* This function may be called repeatedly for an input file consisting
* of several contiguous gzip'ed members.
* IN assertions: there is at least one remaining compressed member.
--
2.7.4
From 223be714a9077de8f18e0344407bf5c13d49b2b1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 4 Nov 2016 20:15:42 -0700
Subject: [PATCH 3/3] gzip: minor time stamp cleanups
* NEWS: Document this.
* gzip.c (get_method): Do not warn about MTIME out of range.
This should avoid useless chatter on hosts with 32-bit time_t
after the year 2038 (!).
(do_list): Do not pass junk time stamp to localtime.
(copy_stat): Do not report "time stamp restored" if restoration
fails.
---
NEWS | 11 ++++++-----
gzip.c | 40 +++++++++++++++++++++-------------------
2 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/NEWS b/NEWS
index 6532550..8d0e100 100644
--- a/NEWS
+++ b/NEWS
@@ -4,11 +4,12 @@ GNU gzip NEWS -*- outline -*-
** Bug fixes
- gzip now warns about file time stamps out of gzip range, or out of
- time_t range, instead of silently continuing, sometimes with
- undefined behavior. This affects time stamps before 1970 or after
- 2106, and time stamps after 2038 on 32-bit platforms.
- [bug present since the beginning]
+ When converting time stamps to gzip file format (32-bit unsigned) or
+ to time_t format (system-dependent), gzip now ignores out-of-range
+ values instead of shoehorning them into the destination format,
+ sometimes with undefined behavior. This affects time stamps before
+ 1970 and after 2106, and time stamps after 2038 on platforms with
+ 32-bit signed time_t. [bug present since the beginning]
Support for VMS and Amiga has been removed. It was not working anyway,
and it reportedly caused file name glitches on MS-Windowsish platforms.
diff --git a/gzip.c b/gzip.c
index dccdb89..17f5709 100644
--- a/gzip.c
+++ b/gzip.c
@@ -189,12 +189,17 @@ static int foreground = 0; /* set if program run in foreground */
int save_orig_name; /* set if original name must be saved */
static int last_member; /* set for .zip and .Z files */
static int part_nb; /* number of parts in .gz file */
- struct timespec time_stamp; /* original time stamp (modification time) */
off_t ifile_size; /* input file size, -1 for devices (debug only) */
static char *env; /* contents of GZIP env variable */
static char const *z_suffix; /* default suffix (can be set with --suffix) */
static size_t z_len; /* strlen(z_suffix) */
+/* The original time stamp (modification time). Its tv_nsec component
+ is negative if the original time is unknown or is out of time_t
+ range; the latter can happen on hosts with 32-bit signed time_t
+ because the gzip format's MTIME is 32-bit unsigned. */
+struct timespec time_stamp;
+
/* The set of signals that are caught. */
static sigset_t caught_signals;
@@ -1534,17 +1539,10 @@ local int get_method(in)
stamp |= ((ulg)get_byte()) << 8;
stamp |= ((ulg)get_byte()) << 16;
stamp |= ((ulg)get_byte()) << 24;
- if (stamp != 0 && !no_time)
+ if (!no_time && 0 < stamp && stamp <= TYPE_MAXIMUM (time_t))
{
- if (stamp <= TYPE_MAXIMUM (time_t))
- {
- time_stamp.tv_sec = stamp;
- time_stamp.tv_nsec = 0;
- }
- else
- WARN ((stderr,
- "%s: %s: MTIME %lu out of range for this platform\n",
- program_name, ifname, stamp));
+ time_stamp.tv_sec = stamp;
+ time_stamp.tv_nsec = 0;
}
magic[8] = get_byte (); /* Ignore extra flags. */
@@ -1773,7 +1771,9 @@ local void do_list(ifd, method)
static char const month_abbr[][4]
= { "Jan", "Feb", "Mar", "Apr", "May", "Jun",
"Jul", "Aug", "Sep", "Oct", "Nov", "Dec" };
- struct tm *tm = localtime (&time_stamp.tv_sec);
+ struct tm *tm = (time_stamp.tv_nsec < 0
+ ? NULL
+ : localtime (&time_stamp.tv_sec));
printf ("%5s %08lx ", methods[method], crc);
if (tm)
printf ("%s%3d %02d:%02d ", month_abbr[tm->tm_mon],
@@ -1919,21 +1919,23 @@ local void copy_stat(ifstat)
int r;
#ifndef NO_UTIME
+ bool restoring;
struct timespec timespec[2];
timespec[0] = get_stat_atime (ifstat);
timespec[1] = get_stat_mtime (ifstat);
+ restoring = (decompress && 0 <= time_stamp.tv_nsec
+ && ! (timespec[1].tv_sec == time_stamp.tv_sec
+ && timespec[1].tv_nsec == time_stamp.tv_nsec));
+ if (restoring)
+ timespec[1] = time_stamp;
- if (decompress && 0 <= time_stamp.tv_nsec
- && ! (timespec[1].tv_sec == time_stamp.tv_sec
- && timespec[1].tv_nsec == time_stamp.tv_nsec))
+ if (fdutimens (ofd, ofname, timespec) == 0)
{
- timespec[1] = time_stamp;
- if (verbose > 1) {
+ if (restoring && 1 < verbose) {
fprintf(stderr, "%s: time stamp restored\n", ofname);
}
}
-
- if (fdutimens (ofd, ofname, timespec) != 0)
+ else
{
int e = errno;
WARN ((stderr, "%s: ", program_name));
--
2.7.4
