From 5bee1c56a9630352d022a8307457bbd0d43630de Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Fri, 22 Nov 2019 03:01:49 -0500 Subject: [PATCH] Properly use RAII to reduce complication --- native/jni/magiskboot/bootimg.cpp | 7 +- native/jni/magiskboot/compress.cpp | 107 +++++++++++------------------ native/jni/utils/include/stream.h | 4 +- native/jni/utils/stream.cpp | 13 +--- 4 files changed, 48 insertions(+), 83 deletions(-) diff --git a/native/jni/magiskboot/bootimg.cpp b/native/jni/magiskboot/bootimg.cpp index 1e8d3bffe..279a6a008 100644 --- a/native/jni/magiskboot/bootimg.cpp +++ b/native/jni/magiskboot/bootimg.cpp @@ -28,9 +28,10 @@ static void decompress(format_t type, int fd, const void *in, size_t size) { static int64_t compress(format_t type, int fd, const void *in, size_t size) { auto prev = lseek(fd, 0, SEEK_CUR); - unique_ptr ptr(get_encoder(type, open_stream(fd))); - ptr->write(in, size); - ptr->close(); + { + unique_ptr ptr(get_encoder(type, open_stream(fd))); + ptr->write(in, size); + } auto now = lseek(fd, 0, SEEK_CUR); return now - prev; } diff --git a/native/jni/magiskboot/compress.cpp b/native/jni/magiskboot/compress.cpp index 0694766ba..232dfbbe8 100644 --- a/native/jni/magiskboot/compress.cpp +++ b/native/jni/magiskboot/compress.cpp @@ -22,7 +22,6 @@ using namespace std; #define bwrite filter_stream::write -#define bclose filter_stream::close constexpr size_t CHUNK = 0x40000; constexpr size_t LZ4_UNCOMPRESSED = 0x800000; @@ -35,26 +34,26 @@ public: int read(void *buf, size_t len) final { return stream::read(buf, len); } - - int close() final { - finish(); - return bclose(); - } - -protected: - // If finish is overridden, destroy should be called in the destructor - virtual void finish() {} - void destroy() { if (fp) finish(); } }; class gz_strm : public cpr_stream { public: - ~gz_strm() override { destroy(); } - int write(const void *buf, size_t len) override { return len ? write(buf, len, Z_NO_FLUSH) : 0; } + ~gz_strm() override { + write(nullptr, 0, Z_FINISH); + switch(mode) { + case DECODE: + inflateEnd(&strm); + break; + case ENCODE: + deflateEnd(&strm); + break; + } + } + protected: enum mode_t { DECODE, @@ -72,18 +71,6 @@ protected: } } - void finish() override { - write(nullptr, 0, Z_FINISH); - switch(mode) { - case DECODE: - inflateEnd(&strm); - break; - case ENCODE: - deflateEnd(&strm); - break; - } - } - private: z_stream strm; uint8_t outbuf[CHUNK]; @@ -125,12 +112,22 @@ public: class bz_strm : public cpr_stream { public: - ~bz_strm() override { destroy(); } - int write(const void *buf, size_t len) override { return len ? write(buf, len, BZ_RUN) : 0; } + ~bz_strm() override { + switch(mode) { + case DECODE: + BZ2_bzDecompressEnd(&strm); + break; + case ENCODE: + write(nullptr, 0, BZ_FINISH); + BZ2_bzCompressEnd(&strm); + break; + } + } + protected: enum mode_t { DECODE, @@ -148,18 +145,6 @@ protected: } } - void finish() override { - switch(mode) { - case DECODE: - BZ2_bzDecompressEnd(&strm); - break; - case ENCODE: - write(nullptr, 0, BZ_FINISH); - BZ2_bzCompressEnd(&strm); - break; - } - } - private: bz_stream strm; char outbuf[CHUNK]; @@ -201,12 +186,15 @@ public: class lzma_strm : public cpr_stream { public: - ~lzma_strm() override { destroy(); } - int write(const void *buf, size_t len) override { return len ? write(buf, len, LZMA_RUN) : 0; } + ~lzma_strm() override { + write(nullptr, 0, LZMA_FINISH); + lzma_end(&strm); + } + protected: enum mode_t { DECODE, @@ -238,11 +226,6 @@ protected: } } - void finish() override { - write(nullptr, 0, LZMA_FINISH); - lzma_end(&strm); - } - private: lzma_stream strm; uint8_t outbuf[CHUNK]; @@ -266,17 +249,17 @@ private: class lzma_decoder : public lzma_strm { public: - lzma_decoder(FILE *fp) : lzma_strm(DECODE, fp) {} + explicit lzma_decoder(FILE *fp) : lzma_strm(DECODE, fp) {} }; class xz_encoder : public lzma_strm { public: - xz_encoder(FILE *fp) : lzma_strm(ENCODE_XZ, fp) {} + explicit xz_encoder(FILE *fp) : lzma_strm(ENCODE_XZ, fp) {} }; class lzma_encoder : public lzma_strm { public: - lzma_encoder(FILE *fp) : lzma_strm(ENCODE_LZMA, fp) {} + explicit lzma_encoder(FILE *fp) : lzma_strm(ENCODE_LZMA, fp) {} }; class LZ4F_decoder : public cpr_stream { @@ -340,12 +323,6 @@ public: LZ4F_createCompressionContext(&ctx, LZ4F_VERSION); } - ~LZ4F_encoder() override { - destroy(); - LZ4F_freeCompressionContext(ctx); - delete[] outbuf; - } - int write(const void *buf, size_t len) override { auto ret = len; if (!outbuf) @@ -368,10 +345,11 @@ public: return ret; } -protected: - void finish() override { + ~LZ4F_encoder() override { size_t len = LZ4F_compressEnd(ctx, outbuf, outCapacity, nullptr); bwrite(outbuf, len); + LZ4F_freeCompressionContext(ctx); + delete[] outbuf; } private: @@ -466,12 +444,6 @@ public: : cpr_stream(fp), outbuf(new char[LZ4_COMPRESSED]), buf(new char[LZ4_UNCOMPRESSED]), init(false), buf_off(0), in_total(0) {} - ~LZ4_encoder() override { - destroy(); - delete[] outbuf; - delete[] buf; - } - int write(const void *in, size_t size) override { if (!init) { bwrite("\x02\x21\x4c\x18", 4); @@ -510,14 +482,15 @@ public: return true; } -protected: - void finish() override { + ~LZ4_encoder() override { if (buf_off) { int write = LZ4_compress_HC(buf, outbuf, buf_off, LZ4_COMPRESSED, 9); bwrite(&write, sizeof(write)); bwrite(outbuf, write); } bwrite(&in_total, sizeof(in_total)); + delete[] outbuf; + delete[] buf; } private: @@ -608,7 +581,7 @@ void decompress(char *infile, const char *outfile) { LOGE("Decompression error!\n"); } - strm->close(); + strm.reset(nullptr); fclose(in_fp); if (rm_in) @@ -651,7 +624,7 @@ void compress(const char *method, const char *infile, const char *outfile) { LOGE("Compression error!\n"); }; - strm->close(); + strm.reset(nullptr); fclose(in_fp); if (rm_in) diff --git a/native/jni/utils/include/stream.h b/native/jni/utils/include/stream.h index b46d91d6e..3013a0c7b 100644 --- a/native/jni/utils/include/stream.h +++ b/native/jni/utils/include/stream.h @@ -18,7 +18,6 @@ public: virtual int read(void *buf, size_t len); virtual int write(const void *buf, size_t len); virtual off_t seek(off_t off, int whence); - virtual int close(); virtual ~stream() = default; }; @@ -26,11 +25,10 @@ public: class filter_stream : public stream { public: filter_stream(FILE *fp) : fp(fp) {} - ~filter_stream() override { if (fp) close(); } + ~filter_stream() override; int read(void *buf, size_t len) override; int write(const void *buf, size_t len) override; - int close() override; void set_base(FILE *f); template diff --git a/native/jni/utils/stream.cpp b/native/jni/utils/stream.cpp index a28734589..28af1e73a 100644 --- a/native/jni/utils/stream.cpp +++ b/native/jni/utils/stream.cpp @@ -19,9 +19,8 @@ static fpos_t strm_seek(void *v, fpos_t off, int whence) { static int strm_close(void *v) { auto strm = reinterpret_cast(v); - int ret = strm->close(); delete strm; - return ret; + return 0; } FILE *open_stream(stream *strm) { @@ -46,8 +45,8 @@ off_t stream::seek(off_t off, int whence) { return -1; } -int stream::close() { - return 0; +filter_stream::~filter_stream() { + if (fp) fclose(fp); } int filter_stream::read(void *buf, size_t len) { @@ -58,12 +57,6 @@ int filter_stream::write(const void *buf, size_t len) { return fwrite(buf, 1, len, fp); } -int filter_stream::close() { - int ret = fclose(fp); - fp = nullptr; - return ret; -} - void filter_stream::set_base(FILE *f) { if (fp) fclose(fp); fp = f;