Date: Sat, 2 Feb 2002 03:29:17 -0500 From: Mike Barcroft <mike@FreeBSD.ORG> To: "Tim J. Robbins" <tim@robbins.dropbear.id.au> Cc: freebsd-standards@FreeBSD.ORG Subject: Re: pwd -L option Message-ID: <20020202032917.K10222@espresso.q9media.com> In-Reply-To: <20020130181638.A8510@descent.robbins.dropbear.id.au>; from tim@robbins.dropbear.id.au on Wed, Jan 30, 2002 at 06:16:38PM %2B1100 References: <20020130181638.A8510@descent.robbins.dropbear.id.au>
next in thread | previous in thread | raw e-mail | index | archive | help
Tim J. Robbins <tim@robbins.dropbear.id.au> writes:
> This patch adds the -L option to pwd; most of the source changes were taken
> from NetBSD, but adapted so as not to break FreeBSD's realpath utility.
> I added an info line in usage() for realpath while I was at it.
>
> It uses the PWD environment variable that sh, ksh93, etc. export and checks
> to make sure it "points" to the physical CWD.
Good approach.
Warner made some modifications to this utility, so you will have to
merge his changes in.
> Index: pwd/pwd.1
> ===================================================================
> RCS file: /home/ncvs/src/bin/pwd/pwd.1,v
> retrieving revision 1.15
> diff -u -r1.15 pwd.1
> --- pwd/pwd.1 2001/08/15 09:09:36 1.15
> +++ pwd/pwd.1 2002/01/30 07:18:10
> @@ -43,6 +43,7 @@
> .Nd return working directory name
> .Sh SYNOPSIS
> .Nm
> +.Op Fl LP
> .Sh DESCRIPTION
> .Nm Pwd
> writes the absolute pathname of the current working directory to
> @@ -54,17 +55,28 @@
> Consult the
> .Xr builtin 1
> manual page.
> +.Pp
> +The options are as follows:
> +.Bl -tag -width indent
> +.It Fl L
> +Display the logical current working directory.
> +.It Fl P
> +Display the physical current working directory (all symbolic links resolved)
> +(default).
Having two sections enclosed in parens is kind of strange. Can you
break the default part out into its own sentence?
> .Sh DIAGNOSTICS
> .Ex -std
> +.Sh ENVIRONMENT
This section belongs above DIAGNOSTICS; see mdoc(7) for details.
> +Environment variables used by
> +.Nm :
> +.Bl -tag -width PWD
> +.It Ev PWD
> +Logical current working directory.
> .Sh STANDARDS
> The
> .Nm
> utility is expected to be
> .St -p1003.2
> compatible.
Please update this part. We are working on `.St -p1003.1-2001'.
> -The
> -.Fl L
> -flag is not supported.
> .Sh SEE ALSO
> .Xr builtin 1 ,
> .Xr cd 1 ,
> @@ -80,3 +92,9 @@
> However, it can give a different answer in the rare case
> that the current directory or a containing directory was moved after
> the shell descended into it.
> +.Pp
> +The
> +.Fl L
> +option does not work unless the
> +.Ev PWD
> +variable is exported by the shell.
> Index: pwd/pwd.c
> ===================================================================
> RCS file: /home/ncvs/src/bin/pwd/pwd.c,v
> retrieving revision 1.13
> diff -u -r1.13 pwd.c
> --- pwd/pwd.c 2001/05/30 03:28:29 1.13
> +++ pwd/pwd.c 2002/01/30 07:18:11
> @@ -45,13 +45,18 @@
> "$FreeBSD: src/bin/pwd/pwd.c,v 1.13 2001/05/30 03:28:29 imp Exp $";
> #endif /* not lint */
>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> #include <err.h>
> +#include <errno.h>
> #include <limits.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/param.h>
>
> +static char *getcwd_logical __P((char *, size_t));
> int main __P((int, char *[]));
> void usage __P((void));
>
> @@ -63,16 +68,15 @@
> int ch;
> char *p;
> char buf[PATH_MAX];
> + int lflag = 0;
Style bug: initialization in the declaration.
This variable should be called `Lflag'.
>
> - /*
> - * Flags for pwd are a bit strange. The POSIX 1003.2B/D9 document
> - * has an optional -P flag for physical, which is what this program
> - * will produce by default. The logical flag, -L, should fail, as
> - * there's no way to display a logical path after forking.
> - */
> - while ((ch = getopt(argc, argv, "P")) != -1)
> + while ((ch = getopt(argc, argv, "LP")) != -1)
Old bug: realpath(1) silently ignored the -P option.
New bug: realpath(1) accepts the -L option.
> switch (ch) {
> + case 'L':
> + lflag = 1;
> + break;
> case 'P':
> + lflag = 0;
One option shouldn't override another. I recommend you add a `Pflag'
and make sure both are not specified at the end of the while loop. If
they are, usage() is the correct place to go.
> break;
> case '?':
> default:
> @@ -87,7 +91,8 @@
> err(1, "%s", argv[0]);
> (void)printf("%s\n", p);
> } else if (argc == 0) {
> - p = getcwd(NULL, (size_t)0);
> + p = lflag ? getcwd_logical(NULL, (size_t)0) :
> + getcwd(NULL, (size_t)0);
Style bug: continuation of a line only adds 4 spaces not a tab.
The cast to size_t was and still is bogus here. Please remove it/them.
> if (p == NULL)
> err(1, ".");
> (void)printf("%s\n", p);
> @@ -98,10 +103,51 @@
> exit(0);
> }
>
> +static char *
> +getcwd_logical(char *pt, size_t size)
Style bug: ANSI-style prototypes. This is a problem because the rest
of the file uses K&R-style.
Neither of these arguments are needed.
> +{
> + char *pwd;
> + size_t pwdlen;
> + dev_t dev;
> + ino_t ino;
> + struct stat s;
Style bug: variable types should be sorted in order of size.
> +
> + /* Check $PWD -- if it's right, it's fast. */
This is a silly comment. It suggests that the alternative is slow.
There is no alternative.
> + if ((pwd = getenv("PWD")) != NULL && pwd[0] == '/') {
> + if (stat(pwd, &s) != -1) {
> + dev = s.st_dev;
> + ino = s.st_ino;
> + if (stat(".", &s) != -1 && dev == s.st_dev &&
> + ino == s.st_ino) {
> + pwdlen = strlen(pwd);
> + if (pt) {
> + if (!size) {
> + errno = EINVAL;
> + return (NULL);
> + }
> + if (pwdlen + 1 > size) {
> + errno = ERANGE;
> + return (NULL);
> + }
This code path isn't needed. It might be if this were being
considered for libc.
> + } else if ((pt = malloc(pwdlen + 1)) == NULL)
> + return (NULL);
> + (void)memmove(pt, pwd, pwdlen);
> + pt[pwdlen] = '\0';
This part doesn't make much sense. We already trust the environment
variable to be NUL terminated (see strlen() above). I recommend this
be changed to use strdup(3).
> + return (pt);
> + }
> + }
> + } else
> + errno = ENOENT;
> +
> + return (NULL);
> +}
> +
> +
Style bug: one line too many.
> void
> usage()
> {
>
> - (void)fprintf(stderr, "usage: pwd\n");
> + (void)fprintf(stderr, "usage: pwd [-LP]\n");
> + (void)fprintf(stderr, " realpath [file ...]\n");
> exit(1);
> }
An alternative might be to display different usages based on the
program executed.
Best regards,
Mike Barcroft
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-standards" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020202032917.K10222>