[llvm] r291494 - TarWriter: Fix a bug in Ustar header.

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 10 00:39:10 PST 2017


I think that technically many (all?) of these should be EXPECT_EQ. I think
ASSERT_EQ is reserved for errors where we "can't go on" if it fails (e.g.
checking the size of a container is >=5, and then we later need to check
something about vec[4]).

On Mon, Jan 9, 2017 at 2:55 PM, Rui Ueyama via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: ruiu
> Date: Mon Jan  9 16:55:00 2017
> New Revision: 291494
>
> URL: http://llvm.org/viewvc/llvm-project?rev=291494&view=rev
> Log:
> TarWriter: Fix a bug in Ustar header.
>
> If we split a filename into `Name` and `Prefix`, `Prefix` is at most
> 145 bytes. We had a bug that didn't split a path correctly. This bug
> was pointed out by Rafael in the post commit review.
>
> This patch adds a unit test for TarWriter to verify the fix.
>
> Added:
>     llvm/trunk/unittests/Support/TarWriterTest.cpp
> Modified:
>     llvm/trunk/lib/Support/TarWriter.cpp
>     llvm/trunk/unittests/Support/CMakeLists.txt
>
> Modified: llvm/trunk/lib/Support/TarWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> Support/TarWriter.cpp?rev=291494&r1=291493&r2=291494&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/Support/TarWriter.cpp (original)
> +++ llvm/trunk/lib/Support/TarWriter.cpp Mon Jan  9 16:55:00 2017
> @@ -122,7 +122,7 @@ static void writePaxHeader(raw_fd_ostrea
>  static std::pair<StringRef, StringRef> splitPath(StringRef Path) {
>    if (Path.size() <= sizeof(UstarHeader::Name))
>      return {"", Path};
> -  size_t Sep = Path.rfind('/', sizeof(UstarHeader::Name) + 1);
> +  size_t Sep = Path.rfind('/', sizeof(UstarHeader::Prefix) + 1);
>    if (Sep == StringRef::npos)
>      return {"", Path};
>    return {Path.substr(0, Sep), Path.substr(Sep + 1)};
>
> Modified: llvm/trunk/unittests/Support/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
> Support/CMakeLists.txt?rev=291494&r1=291493&r2=291494&view=diff
> ============================================================
> ==================
> --- llvm/trunk/unittests/Support/CMakeLists.txt (original)
> +++ llvm/trunk/unittests/Support/CMakeLists.txt Mon Jan  9 16:55:00 2017
> @@ -43,10 +43,11 @@ add_llvm_unittest(SupportTests
>    SpecialCaseListTest.cpp
>    StringPool.cpp
>    SwapByteOrderTest.cpp
> +  TarWriterTest.cpp
>    TargetParserTest.cpp
> -  Threading.cpp
>    ThreadLocalTest.cpp
>    ThreadPool.cpp
> +  Threading.cpp
>    TimerTest.cpp
>    TypeNameTest.cpp
>    TrailingObjectsTest.cpp
>
> Added: llvm/trunk/unittests/Support/TarWriterTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
> Support/TarWriterTest.cpp?rev=291494&view=auto
> ============================================================
> ==================
> --- llvm/trunk/unittests/Support/TarWriterTest.cpp (added)
> +++ llvm/trunk/unittests/Support/TarWriterTest.cpp Mon Jan  9 16:55:00
> 2017
> @@ -0,0 +1,88 @@
> +//===- llvm/unittest/Support/TarWriterTest.cpp
> ----------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===------------------------------------------------------
> ----------------===//
> +
> +#include "llvm/Support/FileSystem.h"
> +#include "llvm/Support/MemoryBuffer.h"
> +#include "llvm/Support/TarWriter.h"
> +#include "gtest/gtest.h"
> +
> +using namespace llvm;
> +namespace {
> +
> +struct UstarHeader {
> +  char Name[100];
> +  char Mode[8];
> +  char Uid[8];
> +  char Gid[8];
> +  char Size[12];
> +  char Mtime[12];
> +  char Checksum[8];
> +  char TypeFlag;
> +  char Linkname[100];
> +  char Magic[6];
> +  char Version[2];
> +  char Uname[32];
> +  char Gname[32];
> +  char DevMajor[8];
> +  char DevMinor[8];
> +  char Prefix[155];
> +  char Pad[12];
> +};
> +
> +class TarWriterTest : public ::testing::Test {};
> +
> +static UstarHeader create(StringRef Base, StringRef Filename) {
> +  // Create a temporary file.
> +  SmallString<128> Path;
> +  std::error_code EC =
> +      sys::fs::createTemporaryFile("TarWriterTest", "tar", Path);
> +  EXPECT_FALSE((bool)EC);
> +
> +  // Create a tar file.
> +  Expected<std::unique_ptr<TarWriter>> TarOrErr =
> TarWriter::create(Path, Base);
> +  EXPECT_TRUE((bool)TarOrErr);
> +  std::unique_ptr<TarWriter> Tar = std::move(*TarOrErr);
> +  Tar->append(Filename, "contents");
> +  Tar.release();
> +
> +  // Read the tar file.
> +  ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr =
> MemoryBuffer::getFile(Path);
> +  EXPECT_TRUE((bool)MBOrErr);
> +  std::unique_ptr<MemoryBuffer> MB = std::move(*MBOrErr);
> +  sys::fs::remove(Path);
> +  return *reinterpret_cast<const UstarHeader *>(MB->getBufferStart());
> +}
> +
> +TEST_F(TarWriterTest, Basics) {
> +  UstarHeader Hdr = create("base", "file");
> +  ASSERT_EQ("ustar", StringRef(Hdr.Magic));
> +  ASSERT_EQ("00", StringRef(Hdr.Version, 2));
> +  ASSERT_EQ("base/file", StringRef(Hdr.Name));
> +  ASSERT_EQ("00000000010", StringRef(Hdr.Size));
> +}
> +
> +TEST_F(TarWriterTest, LongFilename) {
> +  UstarHeader Hdr1 = create(
> +      "012345678", std::string(99, 'x') + "/" + std::string(44, 'x') +
> "/foo");
> +  ASSERT_EQ("foo", StringRef(Hdr1.Name));
> +  ASSERT_EQ("012345678/" + std::string(99, 'x') + "/" + std::string(44,
> 'x'),
> +            StringRef(Hdr1.Prefix));
> +
> +  UstarHeader Hdr2 = create(
> +      "012345678", std::string(99, 'x') + "/" + std::string(45, 'x') +
> "/foo");
> +  ASSERT_EQ("foo", StringRef(Hdr2.Name));
> +  ASSERT_EQ("012345678/" + std::string(99, 'x') + "/" + std::string(45,
> 'x'),
> +            StringRef(Hdr2.Prefix));
> +
> +  UstarHeader Hdr3 = create(
> +      "012345678", std::string(99, 'x') + "/" + std::string(46, 'x') +
> "/foo");
> +  ASSERT_EQ(std::string(46, 'x') + "/foo", StringRef(Hdr3.Name));
> +  ASSERT_EQ("012345678/" + std::string(99, 'x'), StringRef(Hdr3.Prefix));
> +}
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170110/e7765a2c/attachment.html>


More information about the llvm-commits mailing list