Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
On 13 Aug 2020, at 16:33, Ian Lepore wrote: > On Wed, 2020-08-12 at 09:24 -0700, Rodney W. Grimes wrote: >>> On 12 Aug 2020, at 17:10, Rodney W. Grimes < >>> free...@gndrsh.dnsmgr.net> wrote: > Author: arichardson > Date: Wed Aug 12 15:49:06 2020 > New Revision: 364166 > URL: https://svnweb.freebsd.org/changeset/base/364166 > > Log: > Fix crunchgen usage of mkstemp() > > On Glibc systems mkstemp can only be used once with the same > template > string since it will be modified in-place and no longer > contain any 'X' chars. > It is fine to reuse the same file here but we need to be > explicit and use > open() instead of mkstemp() on the second use. > > While touching this file also avoid a hardcoded /bin/pwd since > that may not > work when building on non-FreeBSD systems. This may cause some grief, as now pwd may use a shell builtin and often shell builtin's return a cwd that is not a true full path, ie it may contain symlink compontents in the path. /bin/sh: # cd /tmp/b # /bin/pwd /tmp/a # pwd /tmp/b # ls -lag /tmp/? lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a /tmp/a: total 17 drwxr-xr-x 2 root wheel2 Aug 12 16:06 . drwxrwxrwt 18 root wheel 248 Aug 12 16:06 .. >>> >>> There's the question of whether that really matters; both values >>> are in >>> some sense correct. But if you want to restore the old behaviour, I >>> believe `env pwd` is the portable way to do so? >> >> You have cut the context, but the code has a comment that >> states it is doing this to remove symbolic links, so this >> change infact undoes something that was being done intentionally. >> >> I do believe also that a "env pwd" would do the right thing >> as well. >> > > Or just use "pwd -P" and avoid invoking multiple programs when the > shell can do all the work. Indeed, my suggestion was solving the wrong problem. r364174 added the -P but also needlessly added env too; -P is part of POSIX so any conforming pwd will support it, builtin or not. Jess ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
On Wed, 2020-08-12 at 09:24 -0700, Rodney W. Grimes wrote: > > On 12 Aug 2020, at 17:10, Rodney W. Grimes < > > free...@gndrsh.dnsmgr.net> wrote: > > > > > > > Author: arichardson > > > > Date: Wed Aug 12 15:49:06 2020 > > > > New Revision: 364166 > > > > URL: https://svnweb.freebsd.org/changeset/base/364166 > > > > > > > > Log: > > > > Fix crunchgen usage of mkstemp() > > > > > > > > On Glibc systems mkstemp can only be used once with the same > > > > template > > > > string since it will be modified in-place and no longer > > > > contain any 'X' chars. > > > > It is fine to reuse the same file here but we need to be > > > > explicit and use > > > > open() instead of mkstemp() on the second use. > > > > > > > > While touching this file also avoid a hardcoded /bin/pwd since > > > > that may not > > > > work when building on non-FreeBSD systems. > > > > > > This may cause some grief, as now pwd may use a shell builtin > > > and often shell builtin's return a cwd that is not a true > > > full path, ie it may contain symlink compontents in the > > > path. > > > > > > /bin/sh: > > > > > > # cd /tmp/b > > > # /bin/pwd > > > /tmp/a > > > # pwd > > > /tmp/b > > > # ls -lag /tmp/? > > > lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a > > > > > > /tmp/a: > > > total 17 > > > drwxr-xr-x 2 root wheel2 Aug 12 16:06 . > > > drwxrwxrwt 18 root wheel 248 Aug 12 16:06 .. > > > > There's the question of whether that really matters; both values > > are in > > some sense correct. But if you want to restore the old behaviour, I > > believe `env pwd` is the portable way to do so? > > You have cut the context, but the code has a comment that > states it is doing this to remove symbolic links, so this > change infact undoes something that was being done intentionally. > > I do believe also that a "env pwd" would do the right thing > as well. > Or just use "pwd -P" and avoid invoking multiple programs when the shell can do all the work. -- Ian ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
> On 12 Aug 2020, at 17:10, Rodney W. Grimes wrote: > > > >> Author: arichardson > >> Date: Wed Aug 12 15:49:06 2020 > >> New Revision: 364166 > >> URL: https://svnweb.freebsd.org/changeset/base/364166 > >> > >> Log: > >> Fix crunchgen usage of mkstemp() > >> > >> On Glibc systems mkstemp can only be used once with the same template > >> string since it will be modified in-place and no longer contain any 'X' > >> chars. > >> It is fine to reuse the same file here but we need to be explicit and use > >> open() instead of mkstemp() on the second use. > >> > >> While touching this file also avoid a hardcoded /bin/pwd since that may > >> not > >> work when building on non-FreeBSD systems. > > > > This may cause some grief, as now pwd may use a shell builtin > > and often shell builtin's return a cwd that is not a true > > full path, ie it may contain symlink compontents in the > > path. > > > > /bin/sh: > > > > # cd /tmp/b > > # /bin/pwd > > /tmp/a > > # pwd > > /tmp/b > > # ls -lag /tmp/? > > lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a > > > > /tmp/a: > > total 17 > > drwxr-xr-x 2 root wheel2 Aug 12 16:06 . > > drwxrwxrwt 18 root wheel 248 Aug 12 16:06 .. > > There's the question of whether that really matters; both values are in > some sense correct. But if you want to restore the old behaviour, I > believe `env pwd` is the portable way to do so? You have cut the context, but the code has a comment that states it is doing this to remove symbolic links, so this change infact undoes something that was being done intentionally. I do believe also that a "env pwd" would do the right thing as well. -- Rod Grimes rgri...@freebsd.org ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
On 12 Aug 2020, at 17:10, Rodney W. Grimes wrote: > > [ Charset UTF-8 unsupported, converting... ] >> Author: arichardson >> Date: Wed Aug 12 15:49:06 2020 >> New Revision: 364166 >> URL: https://svnweb.freebsd.org/changeset/base/364166 >> >> Log: >> Fix crunchgen usage of mkstemp() >> >> On Glibc systems mkstemp can only be used once with the same template >> string since it will be modified in-place and no longer contain any 'X' >> chars. >> It is fine to reuse the same file here but we need to be explicit and use >> open() instead of mkstemp() on the second use. >> >> While touching this file also avoid a hardcoded /bin/pwd since that may not >> work when building on non-FreeBSD systems. > > This may cause some grief, as now pwd may use a shell builtin > and often shell builtin's return a cwd that is not a true > full path, ie it may contain symlink compontents in the > path. > > /bin/sh: > > # cd /tmp/b > # /bin/pwd > /tmp/a > # pwd > /tmp/b > # ls -lag /tmp/? > lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a > > /tmp/a: > total 17 > drwxr-xr-x 2 root wheel2 Aug 12 16:06 . > drwxrwxrwt 18 root wheel 248 Aug 12 16:06 .. There's the question of whether that really matters; both values are in some sense correct. But if you want to restore the old behaviour, I believe `env pwd` is the portable way to do so? Jess ___ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r364166 - head/usr.sbin/crunch/crunchgen
[ Charset UTF-8 unsupported, converting... ]
> Author: arichardson
> Date: Wed Aug 12 15:49:06 2020
> New Revision: 364166
> URL: https://svnweb.freebsd.org/changeset/base/364166
>
> Log:
> Fix crunchgen usage of mkstemp()
>
> On Glibc systems mkstemp can only be used once with the same template
> string since it will be modified in-place and no longer contain any 'X'
> chars.
> It is fine to reuse the same file here but we need to be explicit and use
> open() instead of mkstemp() on the second use.
>
> While touching this file also avoid a hardcoded /bin/pwd since that may not
> work when building on non-FreeBSD systems.
This may cause some grief, as now pwd may use a shell builtin
and often shell builtin's return a cwd that is not a true
full path, ie it may contain symlink compontents in the
path.
/bin/sh:
# cd /tmp/b
# /bin/pwd
/tmp/a
# pwd
/tmp/b
# ls -lag /tmp/?
lrwxr-xr-x 1 root wheel 1 Aug 12 16:06 /tmp/b -> a
/tmp/a:
total 17
drwxr-xr-x 2 root wheel2 Aug 12 16:06 .
drwxrwxrwt 18 root wheel 248 Aug 12 16:06 ..
>
> Reviewed By:brooks
> Differential Revision: https://reviews.freebsd.org/D25990
>
> Modified:
> head/usr.sbin/crunch/crunchgen/crunchgen.c
>
> Modified: head/usr.sbin/crunch/crunchgen/crunchgen.c
> ==
> --- head/usr.sbin/crunch/crunchgen/crunchgen.cWed Aug 12 14:45:31
> 2020(r364165)
> +++ head/usr.sbin/crunch/crunchgen/crunchgen.cWed Aug 12 15:49:06
> 2020(r364166)
> @@ -39,10 +39,13 @@ __FBSDID("$FreeBSD$");
>
> #include
> #include
> +#include
> #include
> +#include
> #include
> #include
> #include
> +#include
> #include
>
> #define CRUNCH_VERSION "0.2"
> @@ -91,6 +94,7 @@ prog_t *progs = NULL;
> char confname[MAXPATHLEN], infilename[MAXPATHLEN];
> char outmkname[MAXPATHLEN], outcfname[MAXPATHLEN], execfname[MAXPATHLEN];
> char tempfname[MAXPATHLEN], cachename[MAXPATHLEN], curfilename[MAXPATHLEN];
> +bool tempfname_initialized = false;
> char outhdrname[MAXPATHLEN] ;/* user-supplied header for *.mk */
> char *objprefix; /* where are the objects ? */
> char *path_make;
> @@ -216,6 +220,7 @@ main(int argc, char **argv)
> snprintf(cachename, sizeof(cachename), "%s.cache", confname);
> snprintf(tempfname, sizeof(tempfname), "%s/crunchgen_%sXX",
> getenv("TMPDIR") ? getenv("TMPDIR") : _PATH_TMP, confname);
> + tempfname_initialized = false;
>
> parse_conf_file();
> if (list_mode)
> @@ -648,8 +653,7 @@ fillin_program(prog_t *p)
>
> /* Determine the actual srcdir (maybe symlinked). */
> if (p->srcdir) {
> - snprintf(line, MAXLINELEN, "cd %s && echo -n `/bin/pwd`",
> - p->srcdir);
> + snprintf(line, MAXLINELEN, "cd %s && pwd", p->srcdir);
> f = popen(line,"r");
> if (!f)
> errx(1, "Can't execute: %s\n", line);
> @@ -721,14 +725,26 @@ fillin_program_objs(prog_t *p, char *path)
>
> /* discover the objs from the srcdir Makefile */
>
> - if ((fd = mkstemp(tempfname)) == -1) {
> - perror(tempfname);
> - exit(1);
> + /*
> + * We reuse the same temporary file name for multiple objects. However,
> + * some libc implementations (such as glibc) return EINVAL if there
> + * are no X characters in the template. This happens after the
> + * first call to mkstemp since the argument is modified in-place.
> + * To avoid this error we use open() instead of mkstemp() after the
> + * call to mkstemp().
> + */
> + if (tempfname_initialized) {
> + if ((fd = open(tempfname, O_CREAT | O_EXCL | O_RDWR, 0600)) ==
> -1) {
> + err(EX_OSERR, "open(%s)", tempfname);
> + }
> + } else if ((fd = mkstemp(tempfname)) == -1) {
> + err(EX_OSERR, "mkstemp(%s)", tempfname);
> }
> + tempfname_initialized = true;
> if ((f = fdopen(fd, "w")) == NULL) {
> - warn("%s", tempfname);
> + warn("fdopen(%s)", tempfname);
> goterror = 1;
> - return;
> + goto out;
> }
> if (p->objvar)
> objvar = p->objvar;
> @@ -763,14 +779,14 @@ fillin_program_objs(prog_t *p, char *path)
> if ((f = popen(line, "r")) == NULL) {
> warn("submake pipe");
> goterror = 1;
> - return;
> + goto out;
> }
>
> while(fgets(line, MAXLINELEN, f)) {
> if (strncmp(line, "OBJS= ", 6)) {
> warnx("make error: %s", line);
> goterror = 1;
> - continue;
> + goto out;
> }
>
> cp = line + 6;
> @@ -793,7 +809,7 @@ fillin_program_objs(prog_t *p, char *path)
> warnx(
