Brandon Williams <bmw...@google.com> writes:
> +/* removes the last path component from 'path' except if 'path' is root */
> +static void strip_last_component(struct strbuf *path)
> +{
> + if (path->len > 1) {
> + char *last_slash = find_last_dir_sep(path->buf);
> + strbuf_setlen(path, last_slash - path->buf);
> + }
> +}
You use find_last_dir_sep() which takes care of "Windows uses
backslash" issue. Is this function expected to be fed something
like "C:\My Files\foo.txt" and more importantly "C:\My Files"? Or
is that handled by a lot higher level up in the callchain? I am
reacting the comparison of path->len and 1 here.
Also is the input expected to be normalized? Are we expected to be
fed something like "/a//b/./c/../d/e" and react sensibly, or is that
handled by a lot higher level up in the callchain?
> +/* gets the next component in 'remaining' and places it in 'next' */
> +static void get_next_component(struct strbuf *next, struct strbuf *remaining)
> +{
> + char *start = NULL;
> + char *end = NULL;
> +
> + strbuf_reset(next);
> +
> + /* look for the next component */
> + /* Skip sequences of multiple path-separators */
> + for (start = remaining->buf; is_dir_sep(*start); start++)
> + /* nothing */;
Style:
; /* nothing */
> + /* Find end of the path component */
> + for (end = start; *end && !is_dir_sep(*end); end++)
> + /* nothing */;
> +
> + strbuf_add(next, start, end - start);
OK, so this was given "///foo/bar" in "remaining" and appended
'foo/' to "next". I.e. deduping of slashes is handled here.
POSIX cares about treating "//" at the very beginning of the path
specially. Is that supposed to be handled here, or by a lot higher
level up in the callchain?
> + /* remove the component from 'remaining' */
> + strbuf_remove(remaining, 0, end - remaining->buf);
> +}
> +
> /* We allow "recursive" symbolic links. Only within reason, though. */
> -#define MAXDEPTH 5
> +#define MAXSYMLINKS 5
>
> /*
> * Return the real path (i.e., absolute path, with symlinks resolved
> @@ -21,7 +51,6 @@ int is_directory(const char *path)
> * absolute_path().) The return value is a pointer to a static
> * buffer.
> *
> * 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
> @@ -33,22 +62,16 @@ int is_directory(const char *path)
> */
> static const char *real_path_internal(const char *path, int die_on_error)
> {
> + static struct strbuf resolved = STRBUF_INIT;
This being 'static' would probably mean that this is not reentrant,
which goes against the title of the patch.
