Subject: bug#24826: closed (Re: bug#24826: [PATCH 1/2] gzip --no-name: avoid spurious warning)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 05 Nov 2016 03:37:02 +0000

Your bug report

#24826: [PATCH 1/2] gzip --no-name: avoid spurious warning

which was filed against the gzip package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
24826: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=24826
GNU Bug Tracking System
Contact [email-masked] with problems

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

Subject: [PATCH 1/2] gzip --no-name: avoid spurious warning
From: Jim Meyering <[email-masked]>
To: [email-masked]
Date: Sun, 30 Oct 2016 07:55:08 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

The first one is a clear bug fix.

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.

>From ed37ad2fe34cb37a1dc1687fb6c441e2ebd2e86b Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Mon, 24 Oct 2016 10:10:04 -0700
Subject: [PATCH 1/2] gzip --no-name: avoid spurious warning

I noticed that while attempting to create gzip's own gzip-compressed
release tarball, gzip would emit this warning and exit with status 2:
gzip: stdin: warning: file time stamp out of range for gzip format
Here is a minimal reproducer: : | gzip --no-name > k
* zip.c (zip): Skip validity check when no_time is set.
* gzip.c (no_time): Make this variable global.
* gzip.h (no_time): Declare it extern.
* tests/timestamp: Add a test to exercise the fix.
Introduced by commit v1.8-6-g51dee92
---
 gzip.c          | 2 +-
 gzip.h          | 1 +
 tests/timestamp | 3 +++
 zip.c           | 4 +++-
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gzip.c b/gzip.c
index 0fca5a3..f990fa2 100644
--- a/gzip.c
+++ b/gzip.c
@@ -173,7 +173,7 @@ static int decompress = 0;   /* decompress (-d) */
 static int force = 0;        /* don't ask questions, compress links (-f) */
 static int keep = 0;         /* keep (don't delete) input files */
 static int no_name = -1;     /* don't save or restore the original file name */
-static int no_time = -1;     /* don't save or restore the original file time */
+       int no_time = -1;     /* don't save or restore the original file time */
 static int recursive = 0;    /* recurse through directories (-r) */
 static int list = 0;         /* list the file contents (-l) */
        int verbose = 0;      /* be verbose (-v) */
diff --git a/gzip.h b/gzip.h
index f298b47..4117306 100644
--- a/gzip.h
+++ b/gzip.h
@@ -199,6 +199,7 @@ typedef int file_t;     /* Do not use stdio */

 extern int exit_code;      /* program exit code */
 extern int verbose;        /* be verbose (-v) */
+extern int no_time;        /* --no-time (-T) */
 extern int quiet;          /* be quiet (-q) */
 extern int level;          /* compression level */
 extern int test;           /* check .z file integrity */
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..cebd719 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 (no_time)
+      stamp = 0;
+    else if (0 < time_stamp.tv_sec && time_stamp.tv_sec <= 0xffffffff)
       stamp = time_stamp.tv_sec;
     else
       {
--
2.7.4

>From 6d0eabd745de16616beda963e0bee662a1664478 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Mon, 24 Oct 2016 09:58:22 -0700
Subject: [PATCH 2/2] gzip: accept --no-time option (undocumented, like -m)

The undocumented -m option has been accepted since the beginning,
but its associated --no-time option has never been accepted, due
to the use of 'T' (not mentioned in shortopts) instead of 'm' in
the definition of longopts.  This made it so an attempt to use
this long option would elicit only a bare "Try `gzip --help' for
more information."
* gzip.c (longopts): Specify 'm' with --no-time, not 'T'.
---
 gzip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gzip.c b/gzip.c
index f990fa2..9e34c80 100644
--- a/gzip.c
+++ b/gzip.c
@@ -285,7 +285,7 @@ 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 */
+    {"no-time",    0, 0, 'm'}, /* 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 */
--
2.7.4