Re: [PATCH v5 1/2] abspath: add a function to resolve paths with missing components
From: Eric Sunshine <hidden>
Date: 2020-12-13 02:36:30
On Sat, Dec 12, 2020 at 7:26 PM brian m. carlson [off-list ref] 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.
quoted hunk ↗ jump to hunk
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 <redacted> ---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`.
quoted hunk ↗ jump to hunk
@@ -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.