longoption was not fixed yet. plz, refer red colored diff.
>From a291b0b3b01ca3f981d36a2a5650cc9fd73887a3 Mon Sep 17 00:00:00 2001 From: Hyejin Kim <hj8...@gmail.com> Date: Mon, 2 Feb 2015 21:28:06 +0900 Subject: [PATCH] mktemp : fix to match longopt with correct opt and check invalid dir path of -p opt --- toys/lsb/mktemp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/toys/lsb/mktemp.c b/toys/lsb/mktemp.c index c1175fe..bcc2e00 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, ">1q(directory)d(tmpdir)p:", TOYFLAG_BIN))+USE_MKTEMP(NEWTOY(mktemp, ">1qd(directory)p(tmpdir):", TOYFLAG_BIN))* config MKTEMP bool "mktemp" @@ -41,10 +41,12 @@ void mktemp_main(void) 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) + if (d_flag ? mkdtemp(tmp) == NULL : mkstemp(tmp) == -1) { + toys.exitval++; + if (!(toys.optflags & FLAG_q)) perror_exit("Failed to create temporary %s", d_flag ? "directory" : "file"); + } xputs(tmp); -- 1.9.1 ------------------------------------------------------------------------------------------------- Date: Sat, 07 Feb 2015 20:02:07 -0600 From: Rob Landley <r...@landley.net> To: toybox@lists.landley.net Subject: Re: [Toybox] [PATCH] mktemp fixes Message-ID: <54d6c39f.1050...@landley.net> Content-Type: text/plain; charset=windows-1252 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
