~sircmpwn/ctools

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH v4] Implemented pwd

Details
Message ID
<20191101213346.32819-1-pedrolorgaramos@tecnico.ulisboa.pt>
DKIM signature
pass
Download raw message
Patch: +109 -3
---
Hi, So I believe I've implemented as you wanted. I have tried to made
very clear that -L flag also returns the physical path and it is there
just not to break existing POSIX scripts. I'm not certain it is the best
approach though as people might prefer scripts to break instead of
malfuncioning. Anyway, it prints a warning when -L is used so the old
scrits won't malfunction quietly.

STATUS           |  2 +-
 doc/meson.build  |  1 +
 doc/pwd.1.scd    | 32 ++++++++++++++++++++++++++++++++
 meson.build      |  3 ++-
 src/pwd.c        | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build |  3 ++-
 test/pwd         | 24 ++++++++++++++++++++++++
 7 files changed, 109 insertions(+), 3 deletions(-)
 create mode 100644 doc/pwd.1.scd
 create mode 100644 src/pwd.c
 create mode 100644 test/pwd

diff --git a/STATUS b/STATUS
index d574963..b4cf480 100644
--- a/STATUS
+++ b/STATUS
@@ -101,7 +101,7 @@ T       pathchk
 T       printf
     W   prs
 T       ps
-T       pwd
+ D      pwd
     W   qalter
     W   qdel
     W   qhold
diff --git a/doc/meson.build b/doc/meson.build
index 56bc8e1..17f3cc2 100644
--- a/doc/meson.build
+++ b/doc/meson.build
@@ -21,6 +21,7 @@ man_files = [
 	'logname.1',
 	'nice.1',
 	'nohup.1',
+	'pwd.1',
 	'rmdir.1',
 	'sleep.1',
 	'tee.1',
diff --git a/doc/pwd.1.scd b/doc/pwd.1.scd
new file mode 100644
index 0000000..029d372
--- /dev/null
+++ b/doc/pwd.1.scd
@@ -0,0 +1,32 @@
+pwd(1) "ctools"
+
+# NAME
+
+pwd - returns current working directory.
+
+# SYNOPSIS
+
+*pwd* [ -L | -P ] 
+
+# DESCRIPTION
+
+*pwd* - Prints current working directory to stdout. It always prints physical 
+		pathname regardless, not resolving any symbolic link.
+
+
+# OPTIONS
+
+*-L*
+	Prints physical pathname. This flag is only implemented for POSIX
+	compliante. When this flag is used a warning is printed to stderr.
+
+*-P*
+	Prints physical pathname.
+
+
+# DISCLAIMER
+
+This command is part of ctools and is compatible with POSIX-1.2018, and may
+optionally support XSI extensions. This man page is not intended to be a
+complete reference, and where it disagrees with the specification, the
+specification takes precedence.
diff --git a/meson.build b/meson.build
index 7724a2a..828b270 100644
--- a/meson.build
+++ b/meson.build
@@ -27,12 +27,13 @@ oneshots = [
 	'logname',
 	'nice', # Included in base but only effective under XSI
 	'nohup',
+	'pwd',
 	'rmdir',
 	'sleep',
 	'tee',
 	'true',
 	'tty',
-	'uname',
+	'uname'
 ]
 
 xsi_oneshots = [
diff --git a/src/pwd.c b/src/pwd.c
new file mode 100644
index 0000000..fba93fd
--- /dev/null
+++ b/src/pwd.c
@@ -0,0 +1,47 @@
+#include <getopt.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+static void
+usage(void)
+{
+	fprintf(stderr, "usage: pwd [-L|-P]");
+}
+
+static char *
+get_pwd(void)
+{
+	char *pwd = NULL;
+	return getcwd(pwd,0);
+}
+
+int
+main(int argc, char *argv[])
+{
+	char *lflag_warning = "Warning: This flag also prints the phisical \
+			path and it is here only for POSIX compatibility";
+	char opt;
+
+	char *pwd = get_pwd();
+	if (pwd == NULL) {
+		return 1;
+	}
+
+	while ((opt = getopt(argc, argv, "LP")) != -1) {
+		switch (opt) {
+		case 'P':
+			break;
+		case 'L':
+			fprintf(stdout,"%s\n",lflag_warning);
+			break;
+		default:
+			usage();
+			return 1;
+		}
+	}
+	fprintf(stdout, "%s", pwd);
+	putchar('\n');
+	return 0;
+}
diff --git a/test/meson.build b/test/meson.build
index d8d900a..343a968 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -16,12 +16,13 @@ test_files = [
 	'logname',
 	'nice',
 	'nohup',
+	'pwd',
 	'rmdir',
 	'sleep',
 	'tee',
 	'true',
 	'tty',
-	'uname',
+	'uname'
 ]
 
 xsi_tests = [
diff --git a/test/pwd b/test/pwd
new file mode 100644
index 0000000..de2f5c4
--- /dev/null
+++ b/test/pwd
@@ -0,0 +1,24 @@
+#!/bin/sh
+tool="pwd"
+. "$HARNESS"
+
+mkdir "$TMPDIR"/dir-f
+ln -s "$TMPDIR"/dir-f "$TMPDIR"/dir-s
+
+should_work() (
+    cd "$TMPDIR/dir-f"
+    ct="$(pwd)"
+    [ "$ct" = "$TMPDIR/dir-f" ]
+)
+
+should_handle_symlink() (
+    cd "$TMPDIR/dir-f"
+    ct="$(pwd -L)"
+    [ "$ct" = "$TMPDIR/dir-f" ]
+)
+
+should_resolve_symlink() (
+    cd "$TMPDIR/dir-f"
+    ct="$(pwd -P)"
+    [ "$ct" = "$TMPDIR/dir-f" ]
+)
-- 
2.23.0
Details
Message ID
<BY77YUY6RBQM.YD4CTW5A5PPG@homura>
In-Reply-To
<20191101213346.32819-1-pedrolorgaramos@tecnico.ulisboa.pt> (view parent)
DKIM signature
pass
Download raw message
Thanks! Just some nitpicks and v5 should be mergable.

On Fri Nov 1, 2019 at 9:33 PM Pedro L. Ramos wrote:
> STATUS           |  2 +-
>  doc/meson.build  |  1 +
>  doc/pwd.1.scd    | 32 ++++++++++++++++++++++++++++++++
>  meson.build      |  3 ++-
>  src/pwd.c        | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build |  3 ++-
>  test/pwd         | 24 ++++++++++++++++++++++++

You need to update scdoc(7) with a summary of the new tool.

> --- /dev/null
> +++ b/doc/pwd.1.scd
> @@ -0,0 +1,32 @@
> +pwd(1) "ctools"
> +
> +# NAME
> +
> +pwd - returns current working directory.

s/returns/print/ s/.$//

> +# SYNOPSIS
> +
> +*pwd* [ -L | -P ] 
> +
> +# DESCRIPTION
> +
> +*pwd* - Prints current working directory to stdout. It always prints physical 
> +		pathname regardless, not resolving any symbolic link.


s/*pwd* - // and drop the second sentence. The description describes the
happy path, not the caveats.

> +# OPTIONS
> +
> +*-L*
> +	Prints physical pathname. This flag is only implemented for POSIX
> +	compliante. When this flag is used a warning is printed to stderr.
>
> +*-P*
> +	Prints physical pathname.

Just use:

This flag is implemented for POSIX compatibility and has no effect.

For both flags.

We also need a section here on POSIX compliance. Something like this
will do:

# UNSPECIFIED BEHAVIOR

The POSIX standard specifies the -L and -P flags as having an effect on
the resolution of symlinks in preparing the pathname to print. However,
the ctools implementation of pwd always simply prints the value returned
from *getcwd*(3), regardless of any flags specified.

> diff --git a/meson.build b/meson.build
> index 7724a2a..828b270 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -27,12 +27,13 @@ oneshots = [
>  	'logname',
>  	'nice', # Included in base but only effective under XSI
>  	'nohup',
> +	'pwd',
>  	'rmdir',
>  	'sleep',
>  	'tee',
>  	'true',
>  	'tty',
> -	'uname',
> +	'uname'
>  ]

Don't remove the comma here

>  xsi_oneshots = [
> diff --git a/src/pwd.c b/src/pwd.c
> -%<-
> +static char *
> +get_pwd(void)
> +{
> +	char *pwd = NULL;
> +	return getcwd(pwd,0);
> +}

Why is this its own function? Does not seem necessary

> +int
> +main(int argc, char *argv[])
> +{
> +	char *lflag_warning = "Warning: This flag also prints the phisical \
> +			path and it is here only for POSIX compatibility";

Ditch the warning, the disclaimer in the man page is sufficient

> +	char opt;
> +
> +	char *pwd = get_pwd();
> +	if (pwd == NULL) {
> +		return 1;
> +	}
> +
> +	while ((opt = getopt(argc, argv, "LP")) != -1) {
> +		switch (opt) {
> +		case 'P':
> +			break;
> +		case 'L':
> +			fprintf(stdout,"%s\n",lflag_warning);
> +			break;
> +		default:
> +			usage();
> +			return 1;
> +		}
> +	}

Getopt needs to be the first thing we do.

> +	fprintf(stdout, "%s", pwd);
> +	putchar('\n');
> +	return 0;
> +}

This could be printf("%s\n", pwd);

> diff --git a/test/meson.build b/test/meson.build
> index d8d900a..343a968 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -16,12 +16,13 @@ test_files = [
>  	'logname',
>  	'nice',
>  	'nohup',
> +	'pwd',
>  	'rmdir',
>  	'sleep',
>  	'tee',
>  	'true',
>  	'tty',
> -	'uname',
> +	'uname'
>  ]

Don't remove the comma