From 2726b8a4f6181d0930a96fa91da9733b397e2952 Mon Sep 17 00:00:00 2001 From: Zoltan Szabadka Date: Tue, 6 Oct 2015 11:23:44 +0200 Subject: [PATCH] Encoder fixes. * Remove default constructors. * Initialize bit_cost in histogram.Clear(). * Check fseek result in FileSize. * Replace malloc in BrotliFileIn constructor with "new". * Catch bad_alloc in bro tool. --- enc/brotli_bit_stream.cc | 1 + enc/command.h | 2 -- enc/encode_parallel.cc | 30 ++++++++++++++++++------------ enc/entropy_encode.cc | 3 --- enc/histogram.h | 2 ++ enc/streams.cc | 10 +++------- enc/streams.h | 2 +- tools/bro.cc | 19 ++++++++++++++----- 8 files changed, 39 insertions(+), 30 deletions(-) diff --git a/enc/brotli_bit_stream.cc b/enc/brotli_bit_stream.cc index 0d2b669..898feaa 100644 --- a/enc/brotli_bit_stream.cc +++ b/enc/brotli_bit_stream.cc @@ -339,6 +339,7 @@ std::vector MoveToFrontTransform(const std::vector& v) { std::vector result(v.size()); for (int i = 0; i < v.size(); ++i) { int index = IndexOf(mtf, v[i]); + assert(index >= 0); result[i] = index; MoveToFront(&mtf, index); } diff --git a/enc/command.h b/enc/command.h index b2648db..5609974 100644 --- a/enc/command.h +++ b/enc/command.h @@ -90,8 +90,6 @@ static inline void GetLengthCode(int insertlen, int copylen, int distancecode, } struct Command { - Command() {} - // distance_code is e.g. 0 for same-as-last short code, or 16 for offset 1. Command(int insertlen, int copylen, int copylen_code, int distance_code) : insert_len_(insertlen), copy_len_(copylen) { diff --git a/enc/encode_parallel.cc b/enc/encode_parallel.cc index 3fab609..6fc1707 100644 --- a/enc/encode_parallel.cc +++ b/enc/encode_parallel.cc @@ -39,15 +39,15 @@ namespace brotli { namespace { -void RecomputeDistancePrefixes(std::vector* cmds, +void RecomputeDistancePrefixes(Command* cmds, int num_commands, int num_direct_distance_codes, int distance_postfix_bits) { if (num_direct_distance_codes == 0 && distance_postfix_bits == 0) { return; } - for (int i = 0; i < cmds->size(); ++i) { - Command* cmd = &(*cmds)[i]; + for (int i = 0; i < num_commands; ++i) { + Command* cmd = &cmds[i]; if (cmd->copy_len_ > 0 && cmd->cmd_prefix_ >= 128) { PrefixEncodeCopyDistance(cmd->DistanceCode(), num_direct_distance_codes, @@ -104,7 +104,12 @@ bool WriteMetaBlockParallel(const BrotliParams& params, int num_literals = 0; int max_backward_distance = (1 << params.lgwin) - 16; int dist_cache[4] = { -4, -4, -4, -4 }; - std::vector commands((input_size + 1) >> 1); + Command* commands = static_cast( + malloc(sizeof(Command) * ((input_size + 1) >> 1))); + if (commands == 0) { + delete hashers; + return false; + } CreateBackwardReferences( input_size, input_pos, &input[0], mask, @@ -114,16 +119,15 @@ bool WriteMetaBlockParallel(const BrotliParams& params, hash_type, dist_cache, &last_insert_len, - &commands[0], + commands, &num_commands, &num_literals); delete hashers; - commands.resize(num_commands); if (last_insert_len > 0) { - commands.push_back(Command(last_insert_len)); + commands[num_commands++] = Command(last_insert_len); num_literals += last_insert_len; } - assert(!commands.empty()); + assert(num_commands != 0); // Build the meta-block. MetaBlockSplit mb; @@ -131,17 +135,17 @@ bool WriteMetaBlockParallel(const BrotliParams& params, params.mode == BrotliParams::MODE_FONT ? 12 : 0; int distance_postfix_bits = params.mode == BrotliParams::MODE_FONT ? 1 : 0; int literal_context_mode = utf8_mode ? CONTEXT_UTF8 : CONTEXT_SIGNED; - RecomputeDistancePrefixes(&commands, + RecomputeDistancePrefixes(commands, num_commands, num_direct_distance_codes, distance_postfix_bits); if (params.quality <= 9) { BuildMetaBlockGreedy(&input[0], input_pos, mask, - &commands[0], commands.size(), + commands, num_commands, &mb); } else { BuildMetaBlock(&input[0], input_pos, mask, prev_byte, prev_byte2, - &commands[0], commands.size(), + commands, num_commands, literal_context_mode, &mb); } @@ -173,11 +177,13 @@ bool WriteMetaBlockParallel(const BrotliParams& params, num_direct_distance_codes, distance_postfix_bits, literal_context_mode, - &commands[0], commands.size(), + commands, num_commands, mb, &storage_ix, &storage[0])) { + free(commands); return false; } + free(commands); // If this is not the last meta-block, store an empty metadata // meta-block so that the meta-block will end at a byte boundary. diff --git a/enc/entropy_encode.cc b/enc/entropy_encode.cc index b1dca5c..db986e5 100644 --- a/enc/entropy_encode.cc +++ b/enc/entropy_encode.cc @@ -29,7 +29,6 @@ namespace brotli { namespace { struct HuffmanTree { - HuffmanTree(); HuffmanTree(int count, int16_t left, int16_t right) : total_count_(count), index_left_(left), @@ -40,8 +39,6 @@ struct HuffmanTree { int16_t index_right_or_value_; }; -HuffmanTree::HuffmanTree() {} - // Sort the root nodes, least popular first. bool SortHuffmanTree(const HuffmanTree &v0, const HuffmanTree &v1) { return v0.total_count_ < v1.total_count_; diff --git a/enc/histogram.h b/enc/histogram.h index d513c66..5c82832 100644 --- a/enc/histogram.h +++ b/enc/histogram.h @@ -18,6 +18,7 @@ #define BROTLI_ENC_HISTOGRAM_H_ #include +#include #include #include #include "./command.h" @@ -38,6 +39,7 @@ struct Histogram { void Clear() { memset(data_, 0, sizeof(data_)); total_count_ = 0; + bit_cost_ = std::numeric_limits::infinity(); } void Add(int val) { ++data_[val]; diff --git a/enc/streams.cc b/enc/streams.cc index 0c0d23c..426d73c 100644 --- a/enc/streams.cc +++ b/enc/streams.cc @@ -89,18 +89,14 @@ const void* BrotliMemIn::Read(size_t n, size_t* output) { BrotliFileIn::BrotliFileIn(FILE* f, size_t max_read_size) : f_(f), - buf_(malloc(max_read_size)), - buf_size_(max_read_size) {} + buf_(new char[max_read_size]), + buf_size_(max_read_size) { } BrotliFileIn::~BrotliFileIn() { - if (buf_) free(buf_); + delete[] buf_; } const void* BrotliFileIn::Read(size_t n, size_t* bytes_read) { - if (buf_ == NULL) { - *bytes_read = 0; - return NULL; - } if (n > buf_size_) { n = buf_size_; } else if (n == 0) { diff --git a/enc/streams.h b/enc/streams.h index 4f0c8e7..9fcd980 100644 --- a/enc/streams.h +++ b/enc/streams.h @@ -110,7 +110,7 @@ class BrotliFileIn : public BrotliIn { private: FILE* f_; - void* buf_; + char* buf_; size_t buf_size_; }; diff --git a/tools/bro.cc b/tools/bro.cc index 1e4c2fb..c0a0855 100644 --- a/tools/bro.cc +++ b/tools/bro.cc @@ -177,7 +177,10 @@ int64_t FileSize(char *path) { if (f == NULL) { return -1; } - fseek(f, 0L, SEEK_END); + if (fseek(f, 0L, SEEK_END) != 0) { + fclose(f); + return -1; + } int64_t retval = ftell(f); fclose(f); return retval; @@ -209,10 +212,16 @@ int main(int argc, char** argv) { brotli::BrotliParams params; params.lgwin = lgwin; params.quality = quality; - brotli::BrotliFileIn in(fin, 1 << 16); - brotli::BrotliFileOut out(fout); - if (!BrotliCompress(params, &in, &out)) { - fprintf(stderr, "compression failed\n"); + try { + brotli::BrotliFileIn in(fin, 1 << 16); + brotli::BrotliFileOut out(fout); + if (!BrotliCompress(params, &in, &out)) { + fprintf(stderr, "compression failed\n"); + unlink(output_path); + exit(1); + } + } catch (std::bad_alloc&) { + fprintf(stderr, "not enough memory\n"); unlink(output_path); exit(1); }