[PATCH] D38149: Fix off-by-one error in TarWriter.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/Support/TarWriter.cpp:128
+// Otherwise, returns false.
+static bool splitUstar(StringRef Path, std::string &Prefix, std::string &Name) {
+  if (Path.size() < sizeof(UstarHeader::Name)) {
----------------
I don't think we need `std::string` here.  Can't we just use `StringRef` again?  Also, how about just returning `Optional<std::pair<StringRef, StringRef>>`?


================
Comment at: llvm/lib/Support/TarWriter.cpp:130-131
+  if (Path.size() < sizeof(UstarHeader::Name)) {
+    Name = Path;
+    return true;
+  }
----------------
`return std::make_pair(StringRef(), Path);`


================
Comment at: llvm/lib/Support/TarWriter.cpp:134
+
   size_t Sep = Path.rfind('/', sizeof(UstarHeader::Prefix) + 1);
   if (Sep == StringRef::npos)
----------------
What about this Path?

```
<total of 95 a's>/aaaaa/bb
```

in this case we have 95 + 1 + 5 = 101 characters, followed by a `/`, followed by `bb`.

Does this path conform to Ustar header?  You can split it as `(<total of 95 a's>, "aaaaa/bb")` but then you have a `/` in the name.  Is this ok?


https://reviews.llvm.org/D38149