On Sat, Dec 12, 2020 at 7:26 PM brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> ---
> 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.
