<div dir="ltr"><div dir="ltr"></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 20, 2019 at 10:54 AM Owen Reynolds via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">gbreynoo marked 2 inline comments as done.<br>
gbreynoo added a comment.<br>
<br>
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 `../`.<br></blockquote><div><br></div><div>You can easily remove them using remove_dots().<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
================<br>
Comment at: lib/Object/ArchiveWriter.cpp:499-500<br>
 std::string computeArchiveRelativePath(StringRef From, StringRef To) {<br>
-  if (sys::path::is_absolute(From) || sys::path::is_absolute(To))<br>
-    return To;<br>
+  auto getDotlessAbsolutePath =<br>
+      [](SmallVectorImpl<char> &P) -> std::error_code {<br>
+    std::error_code Ret = sys::fs::make_absolute(P);<br>
----------------<br>
ruiu wrote:<br>
> This lambda doesn't capture anything, you probably should consider moving this outside of this function.<br>
The lambda was to keep it local to this function, i'd rather keep it unless others really disagree. <br>
<br>
<br>
================<br>
Comment at: lib/Object/ArchiveWriter.cpp:514-515<br>
<br>
-  StringRef DirFrom = sys::path::parent_path(From);<br>
-  auto FromI = sys::path::begin(DirFrom);<br>
-  auto ToI = sys::path::begin(To);<br>
+  SmallString<128> PathTo = To;<br>
+  SmallString<128> DirFrom = sys::path::parent_path(From);<br>
+  if (getDotlessAbsolutePath(PathTo) || getDotlessAbsolutePath(DirFrom))<br>
----------------<br>
ruiu wrote:<br>
> 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.<br>
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. <br>
<br>
<br>
<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D59491/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59491/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D59491" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59491</a><br>
<br>
<br>
<br>
</blockquote></div></div>