Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 3 Feb 2002 17:45:11 -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:  <20020203174511.A6496@espresso.q9media.com>
In-Reply-To: <20020202210237.A2326@descent.robbins.dropbear.id.au>; from tim@robbins.dropbear.id.au on Sat, Feb 02, 2002 at 09:02:37PM %2B1100
References:  <20020130181638.A8510@descent.robbins.dropbear.id.au> <20020202032917.K10222@espresso.q9media.com> <20020202210237.A2326@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:
> Here's a new patch that I hope solves the problems you found with my
> previous one. I have just a few comments, though...

Almost... :)

> On Sat, Feb 02, 2002 at 03:29:17AM -0500, Mike Barcroft wrote:
> 
> > Style bug: ANSI-style prototypes.  This is a problem because the rest
> > of the file uses K&R-style.
> 
> usage()'s prototype is also incorrect, then. It is:
> void usage(void);
> 
> It used to have a __P() around its argument list in 4.4BSD. Even then,
> the 1st ed. K&R book says nothing about "void", which is usage()'s return
> type. Is the so-called K&R style that is supposed to be maintained
> documented anywhere?

Oops, sorry.  As Garance pointed out, Warner has apparently already
"fixed" pwd(1) to use ANSI-style prototypes.

> 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/02/02 10:02:31
> @@ -43,6 +43,7 @@
>  .Nd return working directory name
>  .Sh SYNOPSIS
>  .Nm
> +.Op Fl LP

This should be `L | P'.  Similarly, usage() requires this.

>  .Sh DESCRIPTION
>  .Nm Pwd
>  writes the absolute pathname of the current working directory to
> @@ -54,17 +55,35 @@
>  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).
> +This is the default.

> +.Pp
> +.El
> +Only one of
> +.Fl L
> +and
> +.Fl P
> +may be specified.

This is redundant; the usage will now explain this.

> +.Sh ENVIRONMENT
> +Environment variables used by
> +.Nm :
> +.Bl -tag -width PWD
> +.It Ev PWD
> +Logical current working directory.

Missing `.El'.  It looks like it accidentally ended up above.

>  .Sh DIAGNOSTICS
>  .Ex -std
>  .Sh STANDARDS
>  The
>  .Nm
>  utility is expected to be
> -.St -p1003.2
> +.St -p1003.1-2001
>  compatible.

Let's be assertive here and say that this utility does conform.

> -The
> -.Fl L
> -flag is not supported.
>  .Sh SEE ALSO
>  .Xr builtin 1 ,
>  .Xr cd 1 ,
> @@ -80,3 +99,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.14
> diff -u -r1.14 pwd.c
> --- pwd/pwd.c	2002/02/02 06:48:10	1.14
> +++ pwd/pwd.c	2002/02/02 10:02:31
> @@ -45,31 +45,38 @@
>    "$FreeBSD: src/bin/pwd/pwd.c,v 1.14 2002/02/02 06:48:10 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>
>  
> +extern char *__progname;
> +
> +static char *getcwd_logical(void);
>  void usage(void);
>  
>  int
>  main(int argc, char *argv[])
>  {
> +	int Lflag, Pflag;
>  	int ch;
>  	char *p;
>  	char buf[PATH_MAX];
>  
> -	/*
> -	 * 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)
> +	Lflag = Pflag = 0;
> +	while ((ch = getopt(argc, argv, "LP")) != -1)

This isn't quite right yet.  realpath(1) should never call getopt().
It expects no options, so `realpath -L' should return a valid path
ending with `/-L'.

>  		switch (ch) {
> +		case 'L':
> +			++Lflag;

I prefer `Lflag = 1;'.

> +			break;
>  		case 'P':
> +			++Pflag;

Similarly, here.

>  			break;
>  		case '?':
>  		default:
> @@ -78,27 +85,52 @@
>  	argc -= optind;
>  	argv += optind;
>  
> -	if (argc == 1) {
> +	if (strcmp(__progname, "realpath") == 0) {
> +		if (argc != 1 || Lflag || Pflag)
> +			usage();

See above.

>  		p = realpath(argv[0], buf);
>  		if (p == NULL)
>  			err(1, "%s", argv[0]);
>  		(void)printf("%s\n", p);
> -	} else  if (argc == 0) {
> -		p = getcwd(NULL, (size_t)0);
> +	} else {
> +		if (argc != 0 || (Lflag && Pflag))
> +			usage();
> +		p = Lflag ? getcwd_logical() : getcwd(NULL, 0);
>  		if (p == NULL)
>  			err(1, ".");
>  		(void)printf("%s\n", p);
> -	} else {
> -		usage();
>  	}
>  
>  	exit(0);
>  }
>  
>  void
> -usage(void)
> +usage()

Undo this.  See comment above.  (Sorry for the confusion.)

> +{

Style bug: additional vertical space needed here.

> +	if (strcmp (__progname, "realpath") == 0)

                 ^^^ Style bug: unneeded space.

> +		(void)fprintf(stderr, "usage: realpath [file ...]\n");
> +	else
> +		(void)fprintf(stderr, "usage: pwd [-LP]\n");

See above.

> +  	exit(1);
> +}
> +
> +static char *
> +getcwd_logical()
>  {
> +	struct stat log, phy;
> +	char *pwd;
> +
> +	/*
> +	 * Check that $PWD is an absolute logical pathname referring to
> +	 * the current working directory.
> +	 */
> +	if ((pwd = getenv("PWD")) != NULL && *pwd == '/') {
> +		if (stat(pwd, &log) == -1 || stat(".", &phy) == -1)
> +			return (NULL);
> +		if (log.st_dev == phy.st_dev && log.st_ino == phy.st_ino)
> +			return (pwd);
> +	}

Much better.

>  
> -	(void)fprintf(stderr, "usage: pwd\n");
> -	exit(1);
> +	errno = ENOENT;
> +	return (NULL);
>  }

I should be able to commit this after you get these problems fixed.

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?20020203174511.A6496>