Subject: Re: bug#24826: [PATCH 1/2] gzip --no-name: avoid spurious warning
From: Paul Eggert <[email-masked]>
To: Jim Meyering <[email-masked]>, [email-masked]
Date: Fri, 4 Nov 2016 20:36:45 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

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-masked]>
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-masked]>
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-masked]>
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