subject:"Re\: \[Toybox\] \[PATCH\] mktemp\: add \-t and fix behavior."

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-12-05 Thread enh via Toybox

On Wed, Dec 5, 2018 at 10:12 AM enh  wrote:
>
> On Wed, Dec 5, 2018 at 9:57 AM Rob Landley  wrote:
> >
> > On 12/5/18 9:56 AM, enh via Toybox wrote:
> > > 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...
>
> i don't think that's a good idea. we really _shouldn't_ be using
> mktemp(3) *except* to implement mktemp(1)'s -u --- just inlining
> "replace each X with a byte from getrandom" into mktemp(1) makes more
> sense.

(in an lstat(2) loop, that is. which does make me wonder what the
expected behavior of `mktemp -u` is if the directory also isn't
readable...)

>
> > Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-12-05 Thread enh via Toybox

On Wed, Dec 5, 2018 at 9:57 AM Rob Landley  wrote:
>
> On 12/5/18 9:56 AM, enh via Toybox wrote:
> > 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...

i don't think that's a good idea. we really _shouldn't_ be using
mktemp(3) *except* to implement mktemp(1)'s -u --- just inlining
"replace each X with a byte from getrandom" into mktemp(1) makes more
sense.

> Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-12-05 Thread Rob Landley

On 12/5/18 9:56 AM, enh via Toybox wrote:
> 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
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-12-05 Thread enh via Toybox

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.

On Wed, Dec 5, 2018, 03:11 Reverend Homer  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.
>
> On 29/11/2018 03:15, enh via Toybox wrote:
> > 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 000..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.XX 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. | 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. |
> > 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. | 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. | 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.XX, and the default DIR is $TMPDIR if
> set,
> > -else "/tmp".
> > +default TEMPLATE is tmp.XX.
> >   */
> >
> >   #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.XX";
> > +  if (!template) {
> > +toys.optflags |= FLAG_t;
> > +template = "tmp.XX";
> > +  }
> >
> > -  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);
> > +  } else if (toys.optflags & FLAG_d ? !mkdtemp(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
> > Toybox@lists.landley.net
> > http://lists.landley.net/listinfo.cgi/toybox-landley.net
> ___
> Toybox mailing list
> Toybox@lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
>

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-12-05 Thread Reverend Homer


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.

On 29/11/2018 03:15, enh via Toybox wrote:

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 000..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.XX 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. | 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. |
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. | 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. | 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.XX, and the default DIR is $TMPDIR if set,
-else "/tmp".
+default TEMPLATE is tmp.XX.
  */

  #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.XX";
+  if (!template) {
+toys.optflags |= FLAG_t;
+template = "tmp.XX";
+  }

-  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);
+  } else if (toys.optflags & FLAG_d ? !mkdtemp(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
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-11-28 Thread enh via Toybox

On Wed, Nov 28, 2018 at 5:19 PM Rob Landley  wrote:
>
> On 11/28/18 6:15 PM, enh via Toybox wrote:
> > The new tests pass on the host (coreutils 8.28) and with toybox after
> > this patch is applied.
> ...
> > -  if (!template) template = "tmp.XX";
> > +  if (!template) {
> > +toys.optflags |= FLAG_t;
> > +template = "tmp.XX";
> > +  }
> >
> > -  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?

see the final test :-)

(if we were starting again in the 1970s, i'd say "obviously they
should be mutually exclusive", but i assume _someone_ is relying on
the current behavior, so...)

> >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...

yeah, you mentioned that before. if i didn't say so at the time then
yes, i agree that's a great idea.

> Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

2018-11-28 Thread Rob Landley

On 11/28/18 6:15 PM, enh via Toybox wrote:
> The new tests pass on the host (coreutils 8.28) and with toybox after
> this patch is applied.
...
> -  if (!template) template = "tmp.XX";
> +  if (!template) {
> +toys.optflags |= FLAG_t;
> +template = "tmp.XX";
> +  }
> 
> -  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
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

Re: [Toybox] [PATCH] mktemp: add -t and fix behavior.

7 matches

Site Navigation

Mail list logo

Footer information