[PATCH] D28684: [Support/Compression] - Change zlib API to return Error instead of custom status.
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 17 05:17:58 PST 2017
LGTM
George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
> grimar updated this revision to Diff 84645.
> grimar added a comment.
>
> - Addressed review comments.
>
>
> https://reviews.llvm.org/D28684
>
> Files:
> include/llvm/Support/Compression.h
> lib/MC/ELFObjectWriter.cpp
> lib/Object/Decompressor.cpp
> lib/ProfileData/InstrProf.cpp
> lib/Support/Compression.cpp
> unittests/Support/CompressionTest.cpp
>
> Index: unittests/Support/CompressionTest.cpp
> ===================================================================
> --- unittests/Support/CompressionTest.cpp
> +++ unittests/Support/CompressionTest.cpp
> @@ -12,6 +12,7 @@
> //===----------------------------------------------------------------------===//
>
> #include "llvm/Support/Compression.h"
> +#include "llvm/Support/Error.h"
> #include "llvm/ADT/SmallString.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/Config/config.h"
> @@ -26,15 +27,21 @@
> void TestZlibCompression(StringRef Input, zlib::CompressionLevel Level) {
> SmallString<32> Compressed;
> SmallString<32> Uncompressed;
> - EXPECT_EQ(zlib::StatusOK, zlib::compress(Input, Compressed, Level));
> +
> + Error E = zlib::compress(Input, Compressed, Level);
> + EXPECT_FALSE(E);
> + consumeError(std::move(E));
> +
> // Check that uncompressed buffer is the same as original.
> - EXPECT_EQ(zlib::StatusOK,
> - zlib::uncompress(Compressed, Uncompressed, Input.size()));
> + E = zlib::uncompress(Compressed, Uncompressed, Input.size());
> + EXPECT_FALSE(E);
> + consumeError(std::move(E));
> +
> EXPECT_EQ(Input, Uncompressed);
> if (Input.size() > 0) {
> // Uncompression fails if expected length is too short.
> - EXPECT_EQ(zlib::StatusBufferTooShort,
> - zlib::uncompress(Compressed, Uncompressed, Input.size() - 1));
> + E = zlib::uncompress(Compressed, Uncompressed, Input.size() - 1);
> + EXPECT_EQ("zlib error: Z_BUF_ERROR", llvm::toString(std::move(E)));
> }
> }
>
> Index: lib/Support/Compression.cpp
> ===================================================================
> --- lib/Support/Compression.cpp
> +++ lib/Support/Compression.cpp
> @@ -16,14 +16,19 @@
> #include "llvm/ADT/StringRef.h"
> #include "llvm/Config/config.h"
> #include "llvm/Support/Compiler.h"
> +#include "llvm/Support/Error.h"
> #include "llvm/Support/ErrorHandling.h"
> #if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
> #include <zlib.h>
> #endif
>
> using namespace llvm;
>
> #if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
> +static Error createError(StringRef Err) {
> + return make_error<StringError>(Err, inconvertibleErrorCode());
> +}
> +
> static int encodeZlibCompressionLevel(zlib::CompressionLevel Level) {
> switch (Level) {
> case zlib::NoCompression: return 0;
> @@ -34,74 +39,80 @@
> llvm_unreachable("Invalid zlib::CompressionLevel!");
> }
>
> -static zlib::Status encodeZlibReturnValue(int ReturnValue) {
> - switch (ReturnValue) {
> - case Z_OK: return zlib::StatusOK;
> - case Z_MEM_ERROR: return zlib::StatusOutOfMemory;
> - case Z_BUF_ERROR: return zlib::StatusBufferTooShort;
> - case Z_STREAM_ERROR: return zlib::StatusInvalidArg;
> - case Z_DATA_ERROR: return zlib::StatusInvalidData;
> - default: llvm_unreachable("unknown zlib return status!");
> +static StringRef convertZlibCodeToString(int Code) {
> + switch (Code) {
> + case Z_MEM_ERROR:
> + return "zlib error: Z_MEM_ERROR";
> + case Z_BUF_ERROR:
> + return "zlib error: Z_BUF_ERROR";
> + case Z_STREAM_ERROR:
> + return "zlib error: Z_STREAM_ERROR";
> + case Z_DATA_ERROR:
> + return "zlib error: Z_DATA_ERROR";
> + case Z_OK:
> + default:
> + llvm_unreachable("unknown or unexpected zlib status code");
> }
> }
>
> bool zlib::isAvailable() { return true; }
> -zlib::Status zlib::compress(StringRef InputBuffer,
> - SmallVectorImpl<char> &CompressedBuffer,
> - CompressionLevel Level) {
> +
> +Error zlib::compress(StringRef InputBuffer,
> + SmallVectorImpl<char> &CompressedBuffer,
> + CompressionLevel Level) {
> unsigned long CompressedSize = ::compressBound(InputBuffer.size());
> CompressedBuffer.resize(CompressedSize);
> int CLevel = encodeZlibCompressionLevel(Level);
> - Status Res = encodeZlibReturnValue(::compress2(
> - (Bytef *)CompressedBuffer.data(), &CompressedSize,
> - (const Bytef *)InputBuffer.data(), InputBuffer.size(), CLevel));
> + int Res = ::compress2((Bytef *)CompressedBuffer.data(), &CompressedSize,
> + (const Bytef *)InputBuffer.data(), InputBuffer.size(),
> + CLevel);
> // Tell MemorySanitizer that zlib output buffer is fully initialized.
> // This avoids a false report when running LLVM with uninstrumented ZLib.
> __msan_unpoison(CompressedBuffer.data(), CompressedSize);
> CompressedBuffer.resize(CompressedSize);
> - return Res;
> + return Res ? createError(convertZlibCodeToString(Res)) : Error::success();
> }
>
> -zlib::Status zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer,
> - size_t &UncompressedSize) {
> - Status Res = encodeZlibReturnValue(
> +Error zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer,
> + size_t &UncompressedSize) {
> + int Res =
> ::uncompress((Bytef *)UncompressedBuffer, (uLongf *)&UncompressedSize,
> - (const Bytef *)InputBuffer.data(), InputBuffer.size()));
> + (const Bytef *)InputBuffer.data(), InputBuffer.size());
> // Tell MemorySanitizer that zlib output buffer is fully initialized.
> // This avoids a false report when running LLVM with uninstrumented ZLib.
> __msan_unpoison(UncompressedBuffer, UncompressedSize);
> - return Res;
> + return Res ? createError(convertZlibCodeToString(Res)) : Error::success();
> }
>
> -zlib::Status zlib::uncompress(StringRef InputBuffer,
> - SmallVectorImpl<char> &UncompressedBuffer,
> - size_t UncompressedSize) {
> +Error zlib::uncompress(StringRef InputBuffer,
> + SmallVectorImpl<char> &UncompressedBuffer,
> + size_t UncompressedSize) {
> UncompressedBuffer.resize(UncompressedSize);
> - Status Res =
> + Error E =
> uncompress(InputBuffer, UncompressedBuffer.data(), UncompressedSize);
> UncompressedBuffer.resize(UncompressedSize);
> - return Res;
> + return E;
> }
>
> uint32_t zlib::crc32(StringRef Buffer) {
> return ::crc32(0, (const Bytef *)Buffer.data(), Buffer.size());
> }
>
> #else
> bool zlib::isAvailable() { return false; }
> -zlib::Status zlib::compress(StringRef InputBuffer,
> - SmallVectorImpl<char> &CompressedBuffer,
> - CompressionLevel Level) {
> - return zlib::StatusUnsupported;
> -}
> -zlib::Status zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer,
> - size_t &UncompressedSize) {
> - return zlib::StatusUnsupported;
> -}
> -zlib::Status zlib::uncompress(StringRef InputBuffer,
> - SmallVectorImpl<char> &UncompressedBuffer,
> - size_t UncompressedSize) {
> - return zlib::StatusUnsupported;
> +Error zlib::compress(StringRef InputBuffer,
> + SmallVectorImpl<char> &CompressedBuffer,
> + CompressionLevel Level) {
> + llvm_unreachable("zlib::compress is unavailable");
> +}
> +Error zlib::uncompress(StringRef InputBuffer, char *UncompressedBuffer,
> + size_t &UncompressedSize) {
> + llvm_unreachable("zlib::uncompress is unavailable");
> +}
> +Error zlib::uncompress(StringRef InputBuffer,
> + SmallVectorImpl<char> &UncompressedBuffer,
> + size_t UncompressedSize) {
> + llvm_unreachable("zlib::uncompress is unavailable");
> }
> uint32_t zlib::crc32(StringRef Buffer) {
> llvm_unreachable("zlib::crc32 is unavailable");
> Index: lib/ProfileData/InstrProf.cpp
> ===================================================================
> --- lib/ProfileData/InstrProf.cpp
> +++ lib/ProfileData/InstrProf.cpp
> @@ -271,12 +271,12 @@
> }
>
> SmallString<128> CompressedNameStrings;
> - zlib::Status Success =
> - zlib::compress(StringRef(UncompressedNameStrings), CompressedNameStrings,
> - zlib::BestSizeCompression);
> -
> - if (Success != zlib::StatusOK)
> + Error E = zlib::compress(StringRef(UncompressedNameStrings),
> + CompressedNameStrings, zlib::BestSizeCompression);
> + if (E) {
> + consumeError(std::move(E));
> return make_error<InstrProfError>(instrprof_error::compress_failed);
> + }
>
> return WriteStringToResult(CompressedNameStrings.size(),
> CompressedNameStrings);
> @@ -315,9 +315,12 @@
> if (isCompressed) {
> StringRef CompressedNameStrings(reinterpret_cast<const char *>(P),
> CompressedSize);
> - if (zlib::uncompress(CompressedNameStrings, UncompressedNameStrings,
> - UncompressedSize) != zlib::StatusOK)
> + if (Error E =
> + zlib::uncompress(CompressedNameStrings, UncompressedNameStrings,
> + UncompressedSize)) {
> + consumeError(std::move(E));
> return make_error<InstrProfError>(instrprof_error::uncompress_failed);
> + }
> P += CompressedSize;
> NameStrings = StringRef(UncompressedNameStrings.data(),
> UncompressedNameStrings.size());
> Index: lib/Object/Decompressor.cpp
> ===================================================================
> --- lib/Object/Decompressor.cpp
> +++ lib/Object/Decompressor.cpp
> @@ -95,8 +95,5 @@
>
> Error Decompressor::decompress(MutableArrayRef<char> Buffer) {
> size_t Size = Buffer.size();
> - zlib::Status Status = zlib::uncompress(SectionData, Buffer.data(), Size);
> - if (Status != zlib::StatusOK)
> - return createError("decompression failed");
> - return Error::success();
> + return zlib::uncompress(SectionData, Buffer.data(), Size);
> }
> Index: lib/MC/ELFObjectWriter.cpp
> ===================================================================
> --- lib/MC/ELFObjectWriter.cpp
> +++ lib/MC/ELFObjectWriter.cpp
> @@ -32,6 +32,7 @@
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/ELF.h"
> #include "llvm/Support/Endian.h"
> +#include "llvm/Support/Error.h"
> #include "llvm/Support/ErrorHandling.h"
> #include "llvm/Support/StringSaver.h"
> #include <vector>
> @@ -1037,10 +1038,10 @@
> setStream(OldStream);
>
> SmallVector<char, 128> CompressedContents;
> - zlib::Status Success = zlib::compress(
> - StringRef(UncompressedData.data(), UncompressedData.size()),
> - CompressedContents);
> - if (Success != zlib::StatusOK) {
> + if (Error E = zlib::compress(
> + StringRef(UncompressedData.data(), UncompressedData.size()),
> + CompressedContents)) {
> + consumeError(std::move(E));
> getStream() << UncompressedData;
> return;
> }
> Index: include/llvm/Support/Compression.h
> ===================================================================
> --- include/llvm/Support/Compression.h
> +++ include/llvm/Support/Compression.h
> @@ -18,6 +18,7 @@
>
> namespace llvm {
> template <typename T> class SmallVectorImpl;
> +class Error;
> class StringRef;
>
> namespace zlib {
> @@ -29,26 +30,17 @@
> BestSizeCompression
> };
>
> -enum Status {
> - StatusOK,
> - StatusUnsupported, // zlib is unavailable
> - StatusOutOfMemory, // there was not enough memory
> - StatusBufferTooShort, // there was not enough room in the output buffer
> - StatusInvalidArg, // invalid input parameter
> - StatusInvalidData // data was corrupted or incomplete
> -};
> -
> bool isAvailable();
>
> -Status compress(StringRef InputBuffer, SmallVectorImpl<char> &CompressedBuffer,
> - CompressionLevel Level = DefaultCompression);
> +Error compress(StringRef InputBuffer, SmallVectorImpl<char> &CompressedBuffer,
> + CompressionLevel Level = DefaultCompression);
>
> -Status uncompress(StringRef InputBuffer, char *UncompressedBuffer,
> - size_t &UncompressedSize);
> +Error uncompress(StringRef InputBuffer, char *UncompressedBuffer,
> + size_t &UncompressedSize);
>
> -Status uncompress(StringRef InputBuffer,
> - SmallVectorImpl<char> &UncompressedBuffer,
> - size_t UncompressedSize);
> +Error uncompress(StringRef InputBuffer,
> + SmallVectorImpl<char> &UncompressedBuffer,
> + size_t UncompressedSize);
>
> uint32_t crc32(StringRef Buffer);
>
More information about the llvm-commits
mailing list