On 02/07/2015 12:04 PM, enh wrote:
> Use $TMPDIR if set (necessary on Android, where there is no /tmp).
> Include full template in error messages.
> Don't report success on failure with -q.
> Avoid unnecessary allocation.
> Fix "xxxxxx" versus "XXXXXX" confusion.
Apparently I'm not capable of consistently spelling your name in commit
-u arguments.
> diff --git a/toys/lsb/mktemp.c b/toys/lsb/mktemp.c
> index c1175fe..52e53ee 100644
> --- a/toys/lsb/mktemp.c
> +++ b/toys/lsb/mktemp.c
> @@ -12,8 +12,8 @@ config MKTEMP
> help
> usage: mktemp [-dq] [-p DIR] [TEMPLATE]
>
> - Safely create new file and print its name. Default TEMPLATE is
> - /tmp/tmp.XXXXXX and each trailing X is replaced with random char.
> + Safely create a new file and print its name. The default TEMPLATE is
> + tmp.XXXXXX. The default DIR is $TMPDIR, or /tmp if $TMPDIR is not set.
I redid the help text a bit.
> -d, --directory Create directory instead of file
> -p DIR, --tmpdir=DIR Put new file in DIR
> @@ -29,24 +29,27 @@ GLOBALS(
>
> void mktemp_main(void)
> {
> - int d_flag = toys.optflags & FLAG_d;
> - char *tmp;
> + int d_flag = toys.optflags & FLAG_d;
> + char *template = *toys.optargs;
> + int success;
"success" isn't used and that's two int declaration lines anyway.
> - tmp = *toys.optargs;
> -
> - if (!tmp) {
> - if (!TT.tmpdir) TT.tmpdir = "/tmp";
> - tmp = "tmp.xxxxxx";
> + if (!template) {
> + template = "tmp.XXXXXX";
> }
I tend to avoid curly brackets around single lines unless there's
if/else confusion or similar. (Code style thing.)
> - if (TT.tmpdir) tmp = xmprintf("%s/%s", TT.tmpdir ? TT.tmpdir : "/tmp",
> - *toys.optargs ? *toys.optargs : "tmp.XXXXXX");
>
> - if (d_flag ? mkdtemp(tmp) == NULL : mkstemp(tmp) == -1)
> - if (toys.optflags & FLAG_q)
> - perror_exit("Failed to create temporary %s",
> - d_flag ? "directory" : "file");
> + if (!TT.tmpdir) TT.tmpdir = getenv("TMPDIR");
> + if (!TT.tmpdir) TT.tmpdir = "/tmp";
> +
> + snprintf(toybuf, sizeof(toybuf), "%s/%s", TT.tmpdir, template);
So if we _do_ have tmpdir+template combining to be bigger than the old
PATH_MAX, we silently truncate. That seems more like a "throw an error"
situation...
> - xputs(tmp);
> + if (d_flag ? mkdtemp(toybuf) == NULL : mkstemp(toybuf) == -1) {
I try to avoid == 0 comparisons and such, because aero is special in C.
A != 0 test ia a NOP, and for X == 0 we have have !X.
And I've just about stopped using "NULL" entirely. 0 gets typecast to
everything, is shorter, and I actually had something get _confused_ by
being fed a NULL where it wanted a 0 because there was a typecast on it.
(In musl there was a whole argument where they came to the conclusion
NULL had to be 0L so it padded right in printf() without causing the
unpleasant side effects of pointer typecasts, or something like that.)
Neither is a big deal, just an "I tend to wander through after and clean
those up for consistency" sort of heads up.
> + if (toys.optflags & FLAG_q) {
> + toys.exitval = 1;
> + } else {
> + perror_exit("Failed to create temporary %s with template %s/%s",
> + d_flag ? "directory" : "file", TT.tmpdir, template);
> + }
> + }
>
> - if (CFG_TOYBOX_FREE && TT.tmpdir) free(tmp);
> + xputs(toybuf);
Your comment at the top said not to report failure as success, but
you're doing an xputs(toybuf) in the -q failure case anyway? (I added an
else, I assume that's right?)
Rob
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
