On 03/15/2015 08:40 PM, Jim Meyering wrote:
Sounds good.
I too prefer to deprecate, and then (a release or two later) to remove entirely.
OK, thanks, here's a draft patch to do that.
>From 9876b9c6694ffaf12cad355484fa1f5fe482f0ce Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Mon, 16 Mar 2015 14:25:17 -0700
Subject: [PATCH] gzip: make the GZIP env var obsolescent
* NEWS, gzip.1:
* doc/gzip.texi (Environment, Tapes): Document this.
* gzip.c (args): Remove static var; no longer needed now that
'main' frees it. All uses removed.
(ENV_OPTION, shortopts): New constants.
(main): Warn about nontrivial uses of GZIP. Reject dangerous uses.
* tests/gzip-env: New test case.
* tests/Makefile.am (TESTS): Add it.
* util.c (add_envopt): Create new vector instead of adding to old
one. Only use changed.
---
NEWS | 6 ++++
doc/gzip.texi | 43 ++++++++++++--------------
gzip.1 | 46 +++++++++++++++++-----------
gzip.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++------
tests/Makefile.am | 1 +
tests/gzip-env | 43 ++++++++++++++++++++++++++
util.c | 23 +++++++-------
7 files changed, 190 insertions(+), 63 deletions(-)
create mode 100755 tests/gzip-env
diff --git a/NEWS b/NEWS
index 4203f56..120fb93 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,12 @@ GNU gzip NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
+** Changes in behavior
+
+ The GZIP environment variable is now obsolescent; gzip now warns if
+ it is used, and rejects attempts to use dangerous options or operands.
+ You can use an alias or script instead.
+
** Bug fixes
gzip -k -v no longer reports that files are replaced.
diff --git a/doc/gzip.texi b/doc/gzip.texi
index 2f4f1ab..5754aae 100644
--- a/doc/gzip.texi
+++ b/doc/gzip.texi
@@ -469,18 +469,28 @@ complement to @command{tar}, not as a replacement.
@chapter Environment
@cindex Environment
-The environment variable @env{GZIP} can hold a set of default options for
-@command{gzip}. These options are interpreted first and can be overwritten by
-explicit command line parameters. For example:
+The obsolescent environment variable @env{GZIP} can hold a set of
+default options for @command{gzip}. These options are interpreted
+first and can be overwritten by explicit command line parameters. As
+this can cause problems when using scripts, this feature is supported
+only for options that are reasonably likely to not cause too much
+harm, and @command{gzip} warns if it is used. This feature will be
+removed in a future release of @command{gzip}.
+
+You can use an alias or script instead. For example, if
+@command{gzip} is in the directory @samp{/usr/bin} you can prepend
+@file{$HOME/bin} to your @env{PATH} and create an executable script
+@file{$HOME/bin/gzip} containing the following:
@example
-for sh: GZIP="-8v --name"; export GZIP
-for csh: setenv GZIP "-8v --name"
-for MSDOS: set GZIP=-8v --name
+#! /bin/sh
+export PATH=/usr/bin
+exec gzip -9 "$@@"
@end example
-On @abbr{VMS}, the name of the environment variable is @env{GZIP_OPT}, to
-avoid a conflict with the symbol set for invocation of the program.
+On @abbr{VMS}, the name of the obsolescent environment variable is
+@env{GZIP_OPT}, to avoid a conflict with the symbol set for invocation
+of the program.
@node Tapes
@chapter Using @command{gzip} on tapes
@@ -491,21 +501,8 @@ the output with zeroes up to a block boundary. When the data is read and
the whole block is passed to @command{gunzip} for decompression,
@command{gunzip} detects that there is extra trailing garbage after the
compressed data and emits a warning by default if the garbage contains
-nonzero bytes. You have to use the
-@option{--quiet} option to suppress the warning. This option can be set in the
-@env{GZIP} environment variable, as in:
-
-@example
-for sh: GZIP="-q" tar -xfz --block-compress /dev/rst0
-for csh: (setenv GZIP "-q"; tar -xfz --block-compress /dev/rst0)
-@end example
-
-In the above example, @command{gzip} is invoked implicitly by the @option{-z}
-option of @acronym{GNU} @command{tar}. Make sure that the same block
-size (@option{-b}
-option of @command{tar}) is used for reading and writing compressed data on
-tapes. (This example assumes you are using the @acronym{GNU} version of
-@command{tar}.)
+nonzero bytes. You can use the @option{--quiet} option to suppress
+the warning.
@node Problems
@chapter Reporting Bugs
diff --git a/gzip.1 b/gzip.1
index d81c45a..23987e2 100644
--- a/gzip.1
+++ b/gzip.1
@@ -364,17 +364,36 @@ such as tar or zip. GNU tar supports the -z option to invoke gzip
transparently. gzip is designed as a complement to tar, not as a
replacement.
.SH "ENVIRONMENT"
-The environment variable
+The obsolescent environment variable
.B GZIP
can hold a set of default options for
.IR gzip .
-These options are interpreted first and can be overwritten by
-explicit command line parameters. For example:
- for sh: GZIP="-8v --name"; export GZIP
- for csh: setenv GZIP "-8v --name"
- for MSDOS: set GZIP=-8v --name
+These options are interpreted first and can be overwritten by explicit
+command line parameters. As this can cause problems when using
+scripts, this feature is supported only for options that are
+reasonably likely to not cause too much harm, and
+.I gzip
+warns if it is used.
+This feature will be removed in a future release of
+.IR gzip .
+.PP
+You can use an alias or script instead. For example, if
+.I gzip
+is in the directory
+.B /usr/bin
+you can prepend
+.B $HOME/bin
+to your
+.B PATH
+and create an executable script
+.B $HOME/bin/gzip
+containing the following:
-On Vax/VMS, the name of the environment variable is GZIP_OPT, to
+ #! /bin/sh
+ export PATH=/usr/bin
+ exec gzip \-9 "$@"
+
+On VMS, the name of the obsolescent environment variable is GZIP_OPT, to
avoid a conflict with the symbol set for invocation of the program.
.SH "SEE ALSO"
znew(1), zcmp(1), zmore(1), zforce(1), gzexe(1), zip(1), unzip(1), compress(1)
@@ -454,17 +473,8 @@ read and the whole block is passed to
for decompression,
.I gunzip
detects that there is extra trailing garbage after the compressed data
-and emits a warning by default. You have to use the --quiet option to
-suppress the warning. This option can be set in the
-.B GZIP
-environment variable as in:
- for sh: GZIP="-q" tar -xfz --block-compress /dev/rst0
- for csh: (setenv GZIP -q; tar -xfz --block-compr /dev/rst0
-
-In the above example, gzip is invoked implicitly by the -z option of
-GNU tar. Make sure that the same block size (-b option of tar) is used
-for reading and writing compressed data on tapes. (This example
-assumes you are using the GNU version of tar.)
+and emits a warning by default. You can use the --quiet option to
+suppress the warning.
.SH BUGS
The gzip format represents the input size modulo 2^32, so the
--list option reports incorrect uncompressed sizes and compression
diff --git a/gzip.c b/gzip.c
index e69c3e0..19ea909 100644
--- a/gzip.c
+++ b/gzip.c
@@ -188,7 +188,6 @@ 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 **args = NULL; /* argv pointer if GZIP env variable defined */
static char const *z_suffix; /* default suffix (can be set with --suffix) */
static size_t z_len; /* strlen(z_suffix) */
@@ -242,9 +241,15 @@ static int handled_sig[] =
non-character as a pseudo short option, starting with CHAR_MAX + 1. */
enum
{
- PRESUME_INPUT_TTY_OPTION = CHAR_MAX + 1
+ PRESUME_INPUT_TTY_OPTION = CHAR_MAX + 1,
+
+ /* A value greater than all valid long options, used as a flag to
+ distinguish options derived from the GZIP environment variable. */
+ ENV_OPTION
};
+static char const shortopts[] = "ab:cdfhH?klLmMnNqrS:tvVZ123456789";
+
static const struct option longopts[] =
{
/* { name has_arg *flag val } */
@@ -401,7 +406,9 @@ int main (int argc, char **argv)
{
int file_count; /* number of files to process */
size_t proglen; /* length of program_name */
- int optc; /* current option */
+ char **argv_copy;
+ int env_argc;
+ char **env_argv;
EXPAND(argc, argv); /* wild card expansion if necessary */
@@ -415,8 +422,9 @@ int main (int argc, char **argv)
program_name[proglen - 4] = '\0';
/* Add options in GZIP environment variable if there is one */
- env = add_envopt(&argc, &argv, OPTIONS_VAR);
- if (env != NULL) args = argv;
+ argv_copy = argv;
+ env = add_envopt (&env_argc, &argv_copy, OPTIONS_VAR);
+ env_argv = env ? argv_copy : NULL;
#ifndef GNU_STANDARD
# define GNU_STANDARD 1
@@ -440,8 +448,53 @@ int main (int argc, char **argv)
z_suffix = Z_SUFFIX;
z_len = strlen(z_suffix);
- while ((optc = getopt_long (argc, argv, "ab:cdfhH?klLmMnNqrS:tvVZ123456789",
- longopts, (int *)0)) != -1) {
+ while (true) {
+ int optc;
+ int longind = -1;
+
+ if (env_argv)
+ {
+ if (env_argv[optind] && strequ (env_argv[optind], "--"))
+ optc = ENV_OPTION + '-';
+ else
+ {
+ optc = getopt_long (env_argc, env_argv, shortopts, longopts,
+ &longind);
+ if (0 <= optc)
+ optc += ENV_OPTION;
+ else
+ {
+ if (optind != env_argc)
+ {
+ fprintf (stderr,
+ ("%s: %s: non-option in "OPTIONS_VAR
+ " environment variable\n"),
+ program_name, env_argv[optind]);
+ try_help ();
+ }
+
+ /* Wait until here before warning, so that GZIP='-q'
+ doesn't warn. */
+ if (env_argc != 1 && !quiet)
+ fprintf (stderr,
+ ("%s: warning: "OPTIONS_VAR" environment variable"
+ " is deprecated; use an alias or script\n"),
+ program_name);
+
+ /* Start processing ARGC and ARGV instead. */
+ free (env_argv);
+ env_argv = NULL;
+ optind = 1;
+ longind = -1;
+ }
+ }
+ }
+
+ if (!env_argv)
+ optc = getopt_long (argc, argv, shortopts, longopts, &longind);
+ if (optc < 0)
+ break;
+
switch (optc) {
case 'a':
ascii = 1; break;
@@ -474,12 +527,15 @@ int main (int argc, char **argv)
case 'M': /* undocumented, may change later */
no_time = 0; break;
case 'n':
+ case 'n' + ENV_OPTION:
no_name = no_time = 1; break;
case 'N':
+ case 'N' + ENV_OPTION:
no_name = no_time = 0; break;
case PRESUME_INPUT_TTY_OPTION:
presume_input_tty = true; break;
case 'q':
+ case 'q' + ENV_OPTION:
quiet = 1; verbose = 0; break;
case 'r':
#if NO_DIR
@@ -501,6 +557,7 @@ int main (int argc, char **argv)
test = decompress = to_stdout = 1;
break;
case 'v':
+ case 'v' + ENV_OPTION:
verbose++; quiet = 0; break;
case 'V':
version(); do_exit(OK); break;
@@ -513,12 +570,28 @@ int main (int argc, char **argv)
try_help ();
break;
#endif
+ case '1' + ENV_OPTION: case '2' + ENV_OPTION: case '3' + ENV_OPTION:
+ case '4' + ENV_OPTION: case '5' + ENV_OPTION: case '6' + ENV_OPTION:
+ case '7' + ENV_OPTION: case '8' + ENV_OPTION: case '9' + ENV_OPTION:
+ optc -= ENV_OPTION;
+ /* Fall through. */
case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
level = optc - '0';
break;
+
default:
- /* Error message already emitted by getopt_long. */
+ if (ENV_OPTION <= optc && optc != ENV_OPTION + '?')
+ {
+ /* Output a diagnostic, since getopt_long didn't. */
+ fprintf (stderr, "%s: ", program_name);
+ if (0 <= longind)
+ fprintf (stderr, "--%s: ", longopts[longind].name);
+ else
+ fprintf (stderr, "-%c: ", optc - ENV_OPTION);
+ fprintf (stderr, ("option not valid in "OPTIONS_VAR
+ " environment variable\n"));
+ }
try_help ();
}
} /* loop on all arguments */
@@ -1902,8 +1975,6 @@ local void do_exit(exitcode)
in_exit = 1;
free(env);
env = NULL;
- free(args);
- args = NULL;
FREE(inbuf);
FREE(outbuf);
FREE(d_buf);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8dd6436..8252670 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
TESTS = \
+ gzip-env \
helin-segv \
help-version \
hufts \
diff --git a/tests/gzip-env b/tests/gzip-env
new file mode 100755
index 0000000..90ceab3
--- /dev/null
+++ b/tests/gzip-env
@@ -0,0 +1,43 @@
+#!/bin/sh
+# Test the obsolescent GZIP environment variable.
+
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+# limit so don't run it by default.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ..
+
+echo a >exp || framework_failure_
+gzip <exp >in || framework_failure_
+
+fail=0
+GZIP=-qv gzip -d <in >out 2>err || fail=1
+compare exp out || fail=1
+
+for badopt in -- -c --stdout -d --decompress -f --force -h --help -k --keep \
+ -l --list -L --license -r --recursive -Sxxx --suffix=xxx '--suffix xxx' \
+ -t --test -V --version
+do
+ GZIP=$badopt gzip -d <in >out 2>err && fail=1
+done
+
+for goodopt in -n --no-name -N --name -q --quiet -v --verbose \
+ -1 --fast -2 -3 -4 -5 -6 -7 -8 -9 --best
+do
+ GZIP=$goodopt gzip -d <in >out 2>err || fail=1
+ compare exp out || fail=1
+done
+
+Exit $fail
diff --git a/util.c b/util.c
index 881c6ee..f84dfc9 100644
--- a/util.c
+++ b/util.c
@@ -361,11 +361,15 @@ void make_simple_name(name)
} while (p != name);
}
-/* ========================================================================
- * Add an environment variable (if any) before argv, and update argc.
- * Return the expanded environment variable to be freed later, or NULL
- * if no options were added to argv.
- */
+/* Convert the value of the environment variable ENVVAR_NAME
+ to a newly allocated argument vector, and set *ARGCP and *ARGVP
+ to the number of arguments and to the vector, respectively.
+ Make the new vector's zeroth element equal to the old **ARGVP.
+ Return a pointer to the newly allocated string storage.
+
+ If the vector would be empty, do not allocate storage,
+ do not set *ARGCP and *ARGVP, and return NULL. */
+
#define SEPARATOR " \t" /* separators in env variable */
char *add_envopt(
@@ -376,7 +380,6 @@ char *add_envopt(
char *p; /* running pointer through env variable */
char **oargv; /* runs through old argv array */
char **nargv; /* runs through new argv array */
- int oargc = *argcp; /* old argc */
int nargc = 0; /* number of arguments in env variable */
char *env_val;
@@ -396,7 +399,7 @@ char *add_envopt(
free(env_val);
return NULL;
}
- *argcp += nargc;
+ *argcp = nargc + 1;
/* Allocate the new argv array, with an extra element just in case
* the original arg list did not end with a NULL.
*/
@@ -405,9 +408,7 @@ char *add_envopt(
*argvp = nargv;
/* Copy the program name first */
- if (oargc-- < 0)
- gzip_error ("argc<=0");
- *(nargv++) = *(oargv++);
+ *nargv++ = *oargv;
/* Then copy the environment args */
for (p = env_val; nargc > 0; nargc--) {
@@ -416,8 +417,6 @@ char *add_envopt(
while (*p++) ; /* skip over word */
}
- /* Finally copy the old args and add a NULL (usual convention) */
- while (oargc--) *(nargv++) = *(oargv++);
*nargv = NULL;
return env_val;
}
--
2.1.0
