diff mbox series

[v5,1/2] abspath: add a function to resolve paths with missing components

Message ID 20201213002529.542928-2-sandals@crustytoothpaste.net (mailing list archive)
State New, archived
Headers show
Series rev-parse options for absolute or relative paths | expand

Commit Message

brian m. carlson Dec. 13, 2020, 12:25 a.m. UTC
Currently, we have a function to resolve paths, strbuf_realpath.  This
function canonicalizes paths like realpath(3), but permits a trailing
component to be absent from the file system.  In other words, this is
the behavior of the GNU realpath(1) without any arguments.

In the future, we'll need this same behavior, except that we want to
allow for any number of missing trailing components, which is the
behavior of GNU realpath(1) with the -m option.  This is useful because
we'll want to canonicalize a path that may point to a not yet present
path under the .git directory.  For example, a user may want to know
where an arbitrary ref would be stored if it existed in the file system.

Let's refactor strbuf_realpath to move most of the code to an internal
function and then pass it two flags to control its behavior.  We'll add
a strbuf_realpath_forgiving function that has our new behavior, and
leave strbuf_realpath with the older, stricter behavior.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 abspath.c | 64 +++++++++++++++++++++++++++++++++++++++----------------
 cache.h   |  2 ++
 2 files changed, 48 insertions(+), 18 deletions(-)

Comments

Eric Sunshine Dec. 13, 2020, 2:35 a.m. UTC | #1
On Sat, Dec 12, 2020 at 7:26 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> Currently, we have a function to resolve paths, strbuf_realpath.  This
> function canonicalizes paths like realpath(3), but permits a trailing
> component to be absent from the file system.  In other words, this is
> the behavior of the GNU realpath(1) without any arguments.
>
> In the future, we'll need this same behavior, except that we want to
> allow for any number of missing trailing components, which is the
> behavior of GNU realpath(1) with the -m option.  This is useful because
> we'll want to canonicalize a path that may point to a not yet present
> path under the .git directory.  For example, a user may want to know
> where an arbitrary ref would be stored if it existed in the file system.

This improved explanation helps the reader better understand why such
functionality is wanted. Thanks.

> Let's refactor strbuf_realpath to move most of the code to an internal
> function and then pass it two flags to control its behavior.  We'll add
> a strbuf_realpath_forgiving function that has our new behavior, and
> leave strbuf_realpath with the older, stricter behavior.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/abspath.c b/abspath.c
> +#define REALPATH_MANY_MISSING (1 << 0)
> +#define REALPATH_DIE_ON_ERROR (1 << 1)
> +
> +static char *strbuf_realpath_1(struct strbuf *resolved, const char *path,
> +                              int flags)

We normally declare these composed-of-bits "flags" arguments as
`unsigned` rather than `int`.

> @@ -89,7 +85,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
>         if (!*path) {
> -               if (die_on_error)
> +               if (flags & REALPATH_DIE_ON_ERROR)

Nit: If you declare a local variable named `die_on_error`:

    int die_on_error = flags & REALPATH_DIE_ON_ERROR;

then you won't have to touch the existing five places where this
condition is checked, thus making the diff less noisy and the code
(subjectively) a bit easier to read at a glance. Not at all worth a
re-roll, though.

Aside from these minor comments, this version is a nice improvement
over the last.
diff mbox series

Patch

diff --git a/abspath.c b/abspath.c
index 6f15a418bb..39e06b5848 100644
--- a/abspath.c
+++ b/abspath.c
@@ -67,19 +67,15 @@  static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
 #endif
 
 /*
- * Return the real path (i.e., absolute path, with symlinks resolved
- * and extra slashes removed) equivalent to the specified path.  (If
- * you want an absolute path but don't mind links, use
- * absolute_path().)  Places the resolved realpath in the provided strbuf.
- *
- * The directory part of path (i.e., everything up to the last
- * dir_sep) must denote a valid, existing directory, but the last
- * component need not exist.  If die_on_error is set, then die with an
- * informative error message if there is a problem.  Otherwise, return
- * NULL on errors (without generating any output).
+ * If set, any number of trailing components may be missing; otherwise, only one
+ * may be.
  */
-char *strbuf_realpath(struct strbuf *resolved, const char *path,
-		      int die_on_error)
+#define REALPATH_MANY_MISSING (1 << 0)
+/* Should we die if there's an error? */
+#define REALPATH_DIE_ON_ERROR (1 << 1)
+
+static char *strbuf_realpath_1(struct strbuf *resolved, const char *path,
+			       int flags)
 {
 	struct strbuf remaining = STRBUF_INIT;
 	struct strbuf next = STRBUF_INIT;
@@ -89,7 +85,7 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 	struct stat st;
 
 	if (!*path) {
-		if (die_on_error)
+		if (flags & REALPATH_DIE_ON_ERROR)
 			die("The empty string is not a valid path");
 		else
 			goto error_out;
@@ -101,7 +97,7 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 	if (!resolved->len) {
 		/* relative path; can use CWD as the initial resolved path */
 		if (strbuf_getcwd(resolved)) {
-			if (die_on_error)
+			if (flags & REALPATH_DIE_ON_ERROR)
 				die_errno("unable to get current working directory");
 			else
 				goto error_out;
@@ -129,8 +125,9 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 
 		if (lstat(resolved->buf, &st)) {
 			/* error out unless this was the last component */
-			if (errno != ENOENT || remaining.len) {
-				if (die_on_error)
+			if (errno != ENOENT ||
+			   (!(flags & REALPATH_MANY_MISSING) && remaining.len)) {
+				if (flags & REALPATH_DIE_ON_ERROR)
 					die_errno("Invalid path '%s'",
 						  resolved->buf);
 				else
@@ -143,7 +140,7 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 			if (num_symlinks++ > MAXSYMLINKS) {
 				errno = ELOOP;
 
-				if (die_on_error)
+				if (flags & REALPATH_DIE_ON_ERROR)
 					die("More than %d nested symlinks "
 					    "on path '%s'", MAXSYMLINKS, path);
 				else
@@ -153,7 +150,7 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 			len = strbuf_readlink(&symlink, resolved->buf,
 					      st.st_size);
 			if (len < 0) {
-				if (die_on_error)
+				if (flags & REALPATH_DIE_ON_ERROR)
 					die_errno("Invalid symlink '%s'",
 						  resolved->buf);
 				else
@@ -202,6 +199,37 @@  char *strbuf_realpath(struct strbuf *resolved, const char *path,
 	return retval;
 }
 
+/*
+ * Return the real path (i.e., absolute path, with symlinks resolved
+ * and extra slashes removed) equivalent to the specified path.  (If
+ * you want an absolute path but don't mind links, use
+ * absolute_path().)  Places the resolved realpath in the provided strbuf.
+ *
+ * The directory part of path (i.e., everything up to the last
+ * dir_sep) must denote a valid, existing directory, but the last
+ * component need not exist.  If die_on_error is set, then die with an
+ * informative error message if there is a problem.  Otherwise, return
+ * NULL on errors (without generating any output).
+ */
+char *strbuf_realpath(struct strbuf *resolved, const char *path,
+		      int die_on_error)
+{
+	return strbuf_realpath_1(resolved, path,
+				 die_on_error ? REALPATH_DIE_ON_ERROR : 0);
+}
+
+/*
+ * Just like strbuf_realpath, but allows an arbitrary number of path
+ * components to be missing.
+ */
+char *strbuf_realpath_forgiving(struct strbuf *resolved, const char *path,
+				int die_on_error)
+{
+	return strbuf_realpath_1(resolved, path,
+				 ((die_on_error ? REALPATH_DIE_ON_ERROR : 0) |
+				  REALPATH_MANY_MISSING));
+}
+
 char *real_pathdup(const char *path, int die_on_error)
 {
 	struct strbuf realpath = STRBUF_INIT;
diff --git a/cache.h b/cache.h
index 8d279bc110..487ebab8b0 100644
--- a/cache.h
+++ b/cache.h
@@ -1325,6 +1325,8 @@  static inline int is_absolute_path(const char *path)
 int is_directory(const char *);
 char *strbuf_realpath(struct strbuf *resolved, const char *path,
 		      int die_on_error);
+char *strbuf_realpath_forgiving(struct strbuf *resolved, const char *path,
+				int die_on_error);
 char *real_pathdup(const char *path, int die_on_error);
 const char *absolute_path(const char *path);
 char *absolute_pathdup(const char *path);