Re: [PATCH v4 1/2] abspath: add a function to resolve paths with missing components

From: René Scharfe <l.s.r@web.de>
Date: Dec 7, 2020, 17:19 UTC
Message-ID: <c1d1fe44-6a7d-3578-cd89-9aea59c4637a@web.de>
In-Reply-To: <20201206225349.3392790-2-sandals@crustytoothpaste.net>

Am 06.12.20 um 23:53 schrieb brian m. carlson:

Show 31 quoted lines

> We'd like to canonicalize paths such that we can preserve any number of
> trailing components that may be missing.  Let's add a function to do
> that, allowing either one or an unlimited number of components to
> canonicalize, and make strbuf_realpath a wrapper around it that allows
> just one component.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  abspath.c | 13 ++++++++++++-
>  cache.h   |  2 ++
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/abspath.c b/abspath.c
> index 6f15a418bb..e6630b3618 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -80,6 +80,17 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
>   */
>  char *strbuf_realpath(struct strbuf *resolved, const char *path,
>  		      int die_on_error)
> +{
> +	return strbuf_realpath_missing(resolved, path, 0, die_on_error);
> +}
> +
> +/*
> + * Just like strbuf_realpath, but allows specifying how many missing components
> + * are permitted.  If many_missing is true, an arbitrary number of path
> + * components may be missing; otherwise, only the last component may be missing.
> + */
> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
> +			      int many_missing, int die_on_error)

So this uses two 32-bit values to pass two bits.  Hmm.

I find the concept of a "real" path with imaginary components strangely amusing. But perhaps a name like strbuf_resolve_path() would fit better?

Show 12 quoted lines

>  {
>  	struct strbuf remaining = STRBUF_INIT;
>  	struct strbuf next = STRBUF_INIT;
> @@ -129,7 +140,7 @@ 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 (errno != ENOENT || (!many_missing && remaining.len)) {
>  				if (die_on_error)
>  					die_errno("Invalid path '%s'",
>  						  resolved->buf);

So the original code errors out if there is a real error (errno != ENOENT). It also errors out if any component except the last one is missing (errno == ENOENT && remaining.len); that's what the comment is about. This patch adds the ability to ignore ENOENT for all components.

Perhaps convert many_missing and die_on_error into a single flags parameter and implement the flags DIE_ON_ERR and REQUIRE_BASENAME or similar? Callers would be easier to read because such an interface is self-documenting -- provided we find good flag names.

Show 14 quoted lines

> diff --git a/cache.h b/cache.h
> index e986cf4ea9..a1386235fc 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1320,6 +1320,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_missing(struct strbuf *resolved, const char *path,
> +			      int missing_components, 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);
>