Discussion:
[Toybox] [PATCH] mktemp: add -t and fix behavior.
Rob Landley
2018-11-29 01:19:42 UTC
Permalink
The new tests pass on the host (coreutils 8.28) and with toybox after
this patch is applied.
...
- if (!template) template = "tmp.XXXXXX";
+ if (!template) {
+ toys.optflags |= FLAG_t;
+ template = "tmp.XXXXXXXXXX";
+ }
- if (!TT.p) TT.p = getenv("TMPDIR");
+ if (!TT.p || (toys.optflags & FLAG_t)) TT.p = getenv("TMPDIR");
What happens if you specify -p and -t together?
if (!TT.p || !*TT.p) TT.p = "/tmp";
- template = strchr(template, '/') ? xstrdup(template)
- : xmprintf("%s/%s", TT.p, template);
+ // TODO: coreutils cleans paths, so -p /t/// would result in /t/xxx...
+ template = (strchr(template, '/') || !(toys.optflags & (FLAG_p|FLAG_t)))
+ ? xstrdup(template) : xmprintf("%s/%s", TT.p, template);
I used to have xrealpath() but then I had to make xabspath to implement all the
readlink options (the libc one didn't _quite_ let me get in there and specify
the behavior, so I had to reinvent the wheel), and I just use that now.
if (toys.optflags & FLAG_q) toys.exitval = 1;
else perror_exit("Failed to create %s %s/%s",
- d_flag ? "directory" : "file", TT.p, template);
+ toys.optflags & FLAG_d ? "directory" : "file", TT.p, template);
Sigh. I should do a FLAG(d) macro...

Rob
Reverend Homer
2018-12-05 11:11:26 UTC
Permalink
Oops, now I have link-time warning:


ld: generated/obj/mktemp.o: in function `mktemp_main':

mktemp.c:(.text.mktemp_main+0xa9): warning: the use of `mktemp' is dangerous,
better use `mkstemp' or `mkdtemp'


R.H.
The new tests pass on the host (coreutils 8.28) and with toybox after
this patch is applied.
---
tests/mktemp.test | 21 +++++++++++++++++++++
toys/lsb/mktemp.c | 29 ++++++++++++++++-------------
2 files changed, 37 insertions(+), 13 deletions(-)
create mode 100755 tests/mktemp.test
diff --git a/tests/mktemp.test b/tests/mktemp.test
new file mode 100755
index 0000000..d09d2c4
--- /dev/null
+++ b/tests/mktemp.test
@@ -0,0 +1,21 @@
+#!/bin/bash
+
+[ -f testing.sh ] && . testing.sh
+
+#testing "name" "command" "result" "infile" "stdin"
+
+# mktemp by default should use tmp.XXXXXXXXXX as the template,
+# and $TMPDIR as the directory.
+testing "mktemp" "TMPDIR=/t mktemp -u | grep -q
'^/t/tmp\...........$' && echo yes" "yes\n" "" ""
+
+# mktemp with a template should *not* use $TMPDIR.
+testing "mktemp TEMPLATE" "TMPDIR=/t mktemp -u hello.XXXXXXXX | grep
-q '^hello\.........$' && echo yes" "yes\n" "" ""
+
+# mktemp with -t and a template should use $TMPDIR.
+testing "mktemp -t TEMPLATE" "TMPDIR=/t mktemp -u -t hello.XXXXXXXX |
grep -q '^/t/hello\.........$' && echo yes" "yes\n" "" ""
+
+# mktemp with -p DIR and a template should use DIR.
+testing "mktemp -p DIR TEMPLATE" "TMPDIR=/t mktemp -u -p DIR
hello.XXXXXXXX | grep -q '^DIR/hello\.........$' && echo yes" "yes\n"
"" ""
+
+# mktemp -p DIR and -t: -t wins.
+testing "mktemp -p DIR -t TEMPLATE" "TMPDIR=/t mktemp -u -p DIR -t
hello.XXXXXXXX | grep -q '^/t/hello\.........$' && echo yes" "yes\n"
"" ""
diff --git a/toys/lsb/mktemp.c b/toys/lsb/mktemp.c
index 21bb9b3..440cf6b 100644
--- a/toys/lsb/mktemp.c
+++ b/toys/lsb/mktemp.c
@@ -4,7 +4,7 @@
*
* http://refspecs.linuxfoundation.org/LSB_4.1.0/LSB-Core-generic/LSB-Core-generic/mktemp.html
-USE_MKTEMP(NEWTOY(mktemp, ">1uqd(directory)p(tmpdir):", TOYFLAG_BIN))
+USE_MKTEMP(NEWTOY(mktemp, ">1uqd(directory)p(tmpdir):t", TOYFLAG_BIN))
config MKTEMP
bool "mktemp"
@@ -17,11 +17,11 @@ config MKTEMP
-d Create directory instead of file (--directory)
-p Put new file in DIR (--tmpdir)
-q Quiet, no error messages
+ -t Prepend $TMPDIR or /tmp if unset
-u Don't create anything, just print what would be created
Each X in TEMPLATE is replaced with a random printable character. The
- default TEMPLATE is tmp.XXXXXX, and the default DIR is $TMPDIR if set,
- else "/tmp".
+ default TEMPLATE is tmp.XXXXXXXXXX.
*/
#define FOR_mktemp
@@ -33,24 +33,27 @@ GLOBALS(
void mktemp_main(void)
{
- int d_flag = toys.optflags & FLAG_d;
char *template = *toys.optargs;
- if (!template) template = "tmp.XXXXXX";
+ if (!template) {
+ toys.optflags |= FLAG_t;
+ template = "tmp.XXXXXXXXXX";
+ }
- if (!TT.p) TT.p = getenv("TMPDIR");
+ if (!TT.p || (toys.optflags & FLAG_t)) TT.p = getenv("TMPDIR");
if (!TT.p || !*TT.p) TT.p = "/tmp";
- template = strchr(template, '/') ? xstrdup(template)
- : xmprintf("%s/%s", TT.p, template);
+ // TODO: coreutils cleans paths, so -p /t/// would result in /t/xxx...
+ template = (strchr(template, '/') || !(toys.optflags & (FLAG_p|FLAG_t)))
+ ? xstrdup(template) : xmprintf("%s/%s", TT.p, template);
- if (d_flag ? !mkdtemp(template) : mkstemp(template) == -1) {
+ if (toys.optflags & FLAG_u) {
+ mktemp(template);
+ xputs(template);
mkstemp(template) == -1) {
if (toys.optflags & FLAG_q) toys.exitval = 1;
else perror_exit("Failed to create %s %s/%s",
- d_flag ? "directory" : "file", TT.p, template);
- } else {
- if (toys.optflags & FLAG_u) unlink(template);
- xputs(template);
+ toys.optflags & FLAG_d ? "directory" : "file", TT.p, template);
}
if (CFG_TOYBOX_FREE) free(template);
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Rob Landley
2018-12-05 17:57:32 UTC
Permalink
Yes, that's what we've been talking about for days. Search for mktemp and glibc. 
Bionic will warm you at compile time instead, and musl should be silent. 
He's not gonna be the last one to notice, though.

Sigh, I may need to add a mktemp in portability.c...

Rob

Loading...