On 09/26/2013 01:59 PM, Pádraig Brady wrote: > On 09/26/2013 05:36 AM, Jim Meyering wrote: >>> I'll push this soon, unless there are objections >>> to the above "undeprecation" of -p. >> >> Go for it. That seems best. Thank you. > > That -p = --tmpdir aspect is OK. > > But on consideration, erroring on -p,-t is too harsh. > Really -t is just a mode to operate in, > i.e. enforce no '/' in template, and give precedence > to TMPDIR over --tmpdir > Hopefully the attached documentation only patch > suffices to clear things up. > > BTW I noticed this variation which could be > used to generate passwords or something: > > $ mktemp -u -t -p '' XXXXXXXX > L5awccB1 > > However without -t, '/tmp/' is inserted. > I'm inclined to change to not output '/tmp/' here? > > $ mktemp -u -p '' XXXXXXXX > /tmp/L5awccB1
Actually the best way to generate random chars like the above is to avoid -p entirely. So I've made -t consistent and output /tmp/... in the above case. Also I matched the -q option up with the documentation. The documented functionality made sense, which was to suppress only I/O errors, but the existing code suppressed almost all errors, including most usage errors. So I fixed that up in a second patch. Both updated patches are attached, and already pushed. thanks, Pádraig.
>From 8e67c2dec1a595619d540bc3e52c2dfcf61a63a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Sat, 21 Sep 2013 03:33:12 +0100 Subject: [PATCH 1/2] mktemp: synchronize the -p option with docs * src/mktemp.c (usage): Synchronize the -p option description with the logic and info docs. I.E. that -p is just an alias of --tmpdir. Also for consistency treat --tmpdir='' the same with or without -t. I.E. always ignore the --tmpdir option if the param is empty. Fixes http://bugs.gnu.org/15425 --- src/mktemp.c | 22 ++++++++-------------- 1 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/mktemp.c b/src/mktemp.c index 44845c3..41d5763 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -43,7 +43,6 @@ static const char *default_template = "tmp.XXXXXXXXXX"; enum { SUFFIX_OPTION = CHAR_MAX + 1, - TMPDIR_OPTION }; static struct option const longopts[] = @@ -52,7 +51,7 @@ static struct option const longopts[] = {"quiet", no_argument, NULL, 'q'}, {"dry-run", no_argument, NULL, 'u'}, {"suffix", required_argument, NULL, SUFFIX_OPTION}, - {"tmpdir", optional_argument, NULL, TMPDIR_OPTION}, + {"tmpdir", optional_argument, NULL, 'p'}, {GETOPT_HELP_OPTION_DECL}, {GETOPT_VERSION_OPTION_DECL}, {NULL, 0, NULL, 0} @@ -85,20 +84,17 @@ Files are created u+rw, and directories u+rwx, minus umask restrictions.\n\ This option is implied if TEMPLATE does not end in X\n\ "), stdout); fputs (_("\ - --tmpdir[=DIR] interpret TEMPLATE relative to DIR; if DIR is not\n\ + -p DIR, --tmpdir[=DIR] interpret TEMPLATE relative to DIR; if DIR is not\n\ specified, use $TMPDIR if set, else /tmp. With\n\ this option, TEMPLATE must not be an absolute name;\n\ unlike with -t, TEMPLATE may contain slashes, but\n\ mktemp creates only the final component\n\ "), stdout); - fputs ("\n", stdout); fputs (_("\ - -p DIR use DIR as a prefix; implies -t [deprecated]\n\ -t interpret TEMPLATE as a single file name component,\n\ relative to a directory: $TMPDIR, if set; else the\n\ directory specified via -p; else /tmp [deprecated]\n\ "), stdout); - fputs ("\n", stdout); fputs (HELP_OPTION_DESCRIPTION, stdout); fputs (VERSION_OPTION_DESCRIPTION, stdout); emit_ancillary_info (); @@ -195,11 +191,6 @@ main (int argc, char **argv) dry_run = true; break; - case TMPDIR_OPTION: - use_dest_dir = true; - dest_dir_arg = optarg; - break; - case SUFFIX_OPTION: suffix = optarg; break; @@ -283,9 +274,12 @@ main (int argc, char **argv) if (deprecated_t_option) { char *env = getenv ("TMPDIR"); - dest_dir = (env && *env - ? env - : (dest_dir_arg ? dest_dir_arg : "/tmp")); + if (env && *env) + dest_dir = env; + else if (dest_dir_arg && *dest_dir_arg) + dest_dir = dest_dir_arg; + else + dest_dir = "/tmp"; if (last_component (template) != template) error (EXIT_FAILURE, 0, -- 1.7.7.6 >From 0c1d7917f19b4865544ab9f1b13dfcdfd4e65275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Sun, 6 Oct 2013 17:26:51 +0100 Subject: [PATCH 2/2] mktemp: with --quiet, only suppress I/O errors The reason for having a --quiet option is to suppress only some subset of possible errors. The most useful separation here is with usage/internal errors, and errors due to file creation etc. (i.e. I/O errors). * src/mktemp.c (main): Match the --help and info docs and only suppress the file/dir creation error messages. * tests/misc/mktemp.pl: Adjust accordingly. --- src/mktemp.c | 27 ++++++++++----------------- tests/misc/mktemp.pl | 8 ++------ 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/mktemp.c b/src/mktemp.c index 41d5763..05530a3 100644 --- a/src/mktemp.c +++ b/src/mktemp.c @@ -26,7 +26,6 @@ #include "error.h" #include "filenamecat.h" #include "quote.h" -#include "stdio--.h" #include "tempname.h" /* The official name of this program (e.g., no 'g' prefix). */ @@ -147,7 +146,7 @@ main (int argc, char **argv) { char const *dest_dir; char const *dest_dir_arg = NULL; - bool suppress_stderr = false; + bool suppress_file_err = false; int c; unsigned int n_args; char *template; @@ -181,7 +180,7 @@ main (int argc, char **argv) use_dest_dir = true; break; case 'q': - suppress_stderr = true; + suppress_file_err = true; break; case 't': use_dest_dir = true; @@ -205,15 +204,6 @@ main (int argc, char **argv) } } - if (suppress_stderr) - { - /* From here on, redirect stderr to /dev/null. - A diagnostic from getopt_long, above, would still go to stderr. */ - if (!freopen ("/dev/null", "wb", stderr)) - error (EXIT_FAILURE, errno, - _("failed to redirect stderr to /dev/null")); - } - n_args = argc - optind; if (2 <= n_args) { @@ -317,8 +307,9 @@ main (int argc, char **argv) int err = mkdtemp_len (dest_name, suffix_len, x_count, dry_run); if (err != 0) { - error (0, errno, _("failed to create directory via template %s"), - quote (template)); + if (!suppress_file_err) + error (0, errno, _("failed to create directory via template %s"), + quote (template)); status = EXIT_FAILURE; } } @@ -327,8 +318,9 @@ main (int argc, char **argv) int fd = mkstemp_len (dest_name, suffix_len, x_count, dry_run); if (fd < 0 || (!dry_run && close (fd) != 0)) { - error (0, errno, _("failed to create file via template %s"), - quote (template)); + if (!suppress_file_err) + error (0, errno, _("failed to create file via template %s"), + quote (template)); status = EXIT_FAILURE; } } @@ -342,7 +334,8 @@ main (int argc, char **argv) { int saved_errno = errno; remove (dest_name); - error (EXIT_FAILURE, saved_errno, _("write error")); + if (!suppress_file_err) + error (EXIT_FAILURE, saved_errno, _("write error")); } } diff --git a/tests/misc/mktemp.pl b/tests/misc/mktemp.pl index b15b669..d47371c 100755 --- a/tests/misc/mktemp.pl +++ b/tests/misc/mktemp.pl @@ -55,14 +55,12 @@ my @Tests = ( # test-name, [option, option, ...] {OUT=>"expected-output"} # - ['too-many', 'a b', + ['too-many', '-q a b', {ERR=>"$prog: too many templates\n" . "Try '$prog --help' for more information.\n"}, {EXIT => 1} ], - ['too-many-q', '-q a b', {EXIT => 1} ], - ['too-few-x', 'foo.XX', {EXIT => 1}, + ['too-few-x', '-q foo.XX', {EXIT => 1}, {ERR=>"$prog: too few X's in template 'foo.XX'\n"}], - ['too-few-xq', '-q foo.XX', {EXIT => 1} ], ['1f', 'bar.XXXX', {OUT => "bar.ZZZZ\n"}, {OUT_SUBST => 's,\.....$,.ZZZZ,'}, @@ -148,11 +146,9 @@ my @Tests = ['suffix6f', 'aXXXX/b', {EXIT=>1}, {ERR=>"$prog: invalid suffix '/b', contains directory separator\n"}], - ['suffix6f-q', '-q aXXXX/b', {EXIT=>1}], ['suffix7f', '--suffix= aXXXXb', {EXIT=>1}, {ERR=>"$prog: with --suffix, template 'aXXXXb' must end in X\n"}], - ['suffix7f-q', '-q --suffix= aXXXXb', {EXIT=>1}], ['suffix7d', '-d --suffix=aXXXXb ""', {EXIT=>1}, {ERR=>"$prog: with --suffix, template '' must end in X\n"}], -- 1.7.7.6
