[PATCH] D59491: Fix relative thin archive path handling

[Rui Ueyama via llvm-commits]

On Wed, Mar 20, 2019 at 10:54 AM Owen Reynolds via Phabricator <
reviews at reviews.llvm.org> wrote:

> gbreynoo marked 2 inline comments as done.
> gbreynoo added a comment.
>
> The problem with working out the paths is if either are in the form
> `foo/../bar/`. I don't believe your above suggestion would work for paths
> with internal `../`.
>

You can easily remove them using remove_dots().


>
>
> ================
> Comment at: lib/Object/ArchiveWriter.cpp:499-500
>  std::string computeArchiveRelativePath(StringRef From, StringRef To) {
> -  if (sys::path::is_absolute(From) || sys::path::is_absolute(To))
> -    return To;
> +  auto getDotlessAbsolutePath =
> +      [](SmallVectorImpl<char> &P) -> std::error_code {
> +    std::error_code Ret = sys::fs::make_absolute(P);
> ----------------
> ruiu wrote:
> > This lambda doesn't capture anything, you probably should consider
> moving this outside of this function.
> The lambda was to keep it local to this function, i'd rather keep it
> unless others really disagree.
>
>
> ================
> Comment at: lib/Object/ArchiveWriter.cpp:514-515
>
> -  StringRef DirFrom = sys::path::parent_path(From);
> -  auto FromI = sys::path::begin(DirFrom);
> -  auto ToI = sys::path::begin(To);
> +  SmallString<128> PathTo = To;
> +  SmallString<128> DirFrom = sys::path::parent_path(From);
> +  if (getDotlessAbsolutePath(PathTo) || getDotlessAbsolutePath(DirFrom))
> ----------------
> ruiu wrote:
> > Create instances of SmallString and populate their contents as a
> side-effect seems a bit awkward. You could instead make your helper
> function (which is currently named `getDotlessAbsolutePath`) to return an
> std::string.
> Made a change as you've suggested but the use of SmallString is due to
> remove__dots() and I didn't want a second conversion to std::string.
>
>
>
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D59491/new/
>
> https://reviews.llvm.org/D59491
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190320/55d76de8/attachment.html>