[PATCH] Support: add sys::path::canonical

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Jul 31 14:57:45 PDT 2014


> On 2014-Jul-31, at 13:12, Alex Lorenz <arphaman at gmail.com> wrote:
> 
> Fix comment typos
> 
> http://reviews.llvm.org/D4745
> 
> Files:
>  include/llvm/Support/Path.h
>  lib/Support/Path.cpp
>  unittests/Support/Path.cpp
> <D4745.12081.patch>

This logic isn't yet correct for a canonical path, since you're not
resolving symlinks.

> Index: lib/Support/Path.cpp
> ===================================================================
> --- lib/Support/Path.cpp
> +++ lib/Support/Path.cpp
> @@ -531,6 +531,36 @@
>  #endif
>  }
>  
> +void canonical(StringRef Path, SmallVectorImpl<char> &Result) {
> +  Result.clear();
> +  SmallVector<StringRef, 16> Components;
> +
> +  // Split the path into the root and relative parts
> +  // This is needed to ensure that the root path is just one component,
> +  // as the path iterator can represent the root path using more than one
> +  // component.
> +  auto Root = root_path(Path);
> +  auto Relative = Path.substr(Root.size());
> +  if (!Root.empty())
> +    Components.push_back(Root);
> +
> +  for(auto I = begin(Relative), End = end(Relative); I != End; ++I) {
> +    auto Component = *I;
> +    if (Component == ".")
> +      continue;
> +    else if(Component == ".." && !Components.empty() &&
> +            Components.back() != "..") {
> +      // Remove the previous component unless it's root
> +      if (Components.back() != Root)
> +        Components.pop_back();

Worst of all, this will change the file being pointed to if the last
component is a symlink.

> +      continue;
> +    }
> +    Components.push_back(Component);

If you check here for symlinks and resolve them as you go, then the
above logic will be fine.

> +  }
> +  for (const auto &Component : Components)
> +    append(Result, Component);
> +}
> +
>  const StringRef filename(StringRef path) {
>    return *(--end(path));
>  }
> 

Here's some (likely horrible) bash code that I wrote to workaround BSD
readlink not having the related `-f` option.  It's in the wrong
language, and obviously only valid on Unix-like systems, but you can use
it as pseudo-code.

    readlink-f() {
      local input="$1"
      local maxdepth=100
      local depth=0
    
      [ / = "${input:0:1}" ] ||
        input="$PWD/$input"
    
      local output= current= trail="${input:1}"
      while [ -n "$trail" ]; do
        # Ignore leading / in trail.
        while [ / = "${trail:0:1}" ]; do
          trail="${trail:1}"
        done
    
        # Pull off the first element.
        if [ "$trail" = "${trail#*/}" ]; then
          # Also the last element...
          current="$trail"
          trail=
        else
          current="${trail%%/*}"
          trail="${trail#*/}"
        fi
    
        if symlink="$(readlink "$output/$current")"; then
          # Check max depth.
          [ $(( ++depth )) -gt $maxdepth ] &&
            error "$input: exceeded readlink-f max symlink depth of $maxdepth"
    
          # Follow symlink.
          if [ / = "${symlink:0:1}" ]; then
            output=
            trail="${symlink:1}/$trail"
          else
            trail="$symlink/$trail"
          fi
        else
          case "$current" in
            "" | .)
              true
              ;;
            ..)
              [ -n "$output" ] ||
                error "/ has no parent directory"
              output="${output%/*}"
              ;;
            *)
              output="$output/$current"
              ;;
          esac
        fi
      done
    
      echo "${output:-/}"
    }





More information about the llvm-commits mailing list