From be78e51f8ca51c532b16551ec7e476e2fc0886de Mon Sep 17 00:00:00 2001 From: gabime Date: Sun, 9 Feb 2014 01:56:21 +0200 Subject: [PATCH] some effic++ warnings fixes --- include/c11log/details/blocking_queue.h | 165 ++++++++--------- include/c11log/details/fast_oss.h | 2 +- include/c11log/details/line_logger.h | 7 +- include/c11log/logger.h | 12 +- include/c11log/sinks/async_sink.h | 6 +- include/c11log/sinks/file_sinks.h | 232 ++++++++++++------------ 6 files changed, 214 insertions(+), 210 deletions(-) diff --git a/include/c11log/details/blocking_queue.h b/include/c11log/details/blocking_queue.h index 8b44dfeb..a64a39f0 100644 --- a/include/c11log/details/blocking_queue.h +++ b/include/c11log/details/blocking_queue.h @@ -10,104 +10,105 @@ #include #include -namespace c11log { +namespace c11log +{ namespace details { - + template -class blocking_queue { +class blocking_queue +{ public: - using queue_t = std::queue; - using size_type = typename queue_t::size_type; - using clock = std::chrono::system_clock; + using queue_t = std::queue; + using size_type = typename queue_t::size_type; + using clock = std::chrono::system_clock; - explicit blocking_queue(size_type max_size) :max_size_(max_size), q_() - {} - blocking_queue(const blocking_queue&) = delete; - blocking_queue& operator=(const blocking_queue&) = delete; - ~blocking_queue() = default; + explicit blocking_queue(size_type max_size) : + max_size_(max_size), + q_(), + mutex_() + {} + blocking_queue(const blocking_queue&) = delete; + blocking_queue& operator=(const blocking_queue&) = delete; + ~blocking_queue() = default; - size_type size() - { - std::lock_guard lock(mutex_); - return q_.size(); - } + size_type size() { + std::lock_guard lock(mutex_); + return q_.size(); + } - // Push copy of item into the back of the queue. - // If the queue is full, block the calling thread util there is room or timeout have passed. - // Return: false on timeout, true on successful push. - template - bool push(TT&& item, const std::chrono::duration& timeout) - { - std::unique_lock ul(mutex_); - if (q_.size() >= max_size_) - { - if (!item_popped_cond_.wait_until(ul, clock::now() + timeout, [this]() { return this->q_.size() < this->max_size_; })) - return false; - } - q_.push(std::forward(item)); - if (q_.size() <= 1) - { - ul.unlock(); //So the notified thread will have better chance to accuire the lock immediatly.. - item_pushed_cond_.notify_one(); - } - return true; - } + // Push copy of item into the back of the queue. + // If the queue is full, block the calling thread util there is room or timeout have passed. + // Return: false on timeout, true on successful push. + template + bool push(TT&& item, const std::chrono::duration& timeout) { + std::unique_lock ul(mutex_); + if (q_.size() >= max_size_) { + if (!item_popped_cond_.wait_until(ul, clock::now() + timeout, [this]() { + return this->q_.size() < this->max_size_; + })) + return false; + } + q_.push(std::forward(item)); + if (q_.size() <= 1) { + ul.unlock(); //So the notified thread will have better chance to accuire the lock immediatly.. + item_pushed_cond_.notify_one(); + } + return true; + } - // Push copy of item into the back of the queue. - // If the queue is full, block the calling thread until there is room. - template - void push(TT&& item) - { - while (!push(std::forward(item), one_hour)); - } + // Push copy of item into the back of the queue. + // If the queue is full, block the calling thread until there is room. + template + void push(TT&& item) { + while (!push(std::forward(item), one_hour)); + } - // Pop a copy of the front item in the queue into the given item ref. - // If the queue is empty, block the calling thread util there is item to pop or timeout have passed. - // Return: false on timeout , true on successful pop/ - template - bool pop(T& item, const std::chrono::duration& timeout) - { - std::unique_lock ul(mutex_); - if (q_.empty()) - { - if (!item_pushed_cond_.wait_until(ul, clock::now() + timeout, [this]() { return !this->q_.empty(); })) - return false; - } - item = std::move(q_.front()); - q_.pop(); - if (q_.size() >= max_size_ - 1) - { - ul.unlock(); //So the notified thread will have better chance to accuire the lock immediatly.. - item_popped_cond_.notify_one(); - } - return true; - } + // Pop a copy of the front item in the queue into the given item ref. + // If the queue is empty, block the calling thread util there is item to pop or timeout have passed. + // Return: false on timeout , true on successful pop/ + template + bool pop(T& item, const std::chrono::duration& timeout) { + std::unique_lock ul(mutex_); + if (q_.empty()) { + if (!item_pushed_cond_.wait_until(ul, clock::now() + timeout, [this]() { + return !this->q_.empty(); + })) + return false; + } + item = std::move(q_.front()); + q_.pop(); + if (q_.size() >= max_size_ - 1) { + ul.unlock(); //So the notified thread will have better chance to accuire the lock immediatly.. + item_popped_cond_.notify_one(); + } + return true; + } - // Pop a copy of the front item in the queue into the given item ref. - // If the queue is empty, block the calling thread util there is item to pop. - void pop(T& item) - { - while (!pop(item, one_hour)); - } + // Pop a copy of the front item in the queue into the given item ref. + // If the queue is empty, block the calling thread util there is item to pop. + void pop(T& item) { + while (!pop(item, one_hour)); + } - // Clear the queue - void clear() - { - { + // Clear the queue + void clear() { + { std::unique_lock ul(mutex_); queue_t().swap(q_); } - item_popped_cond_.notify_all(); - } + item_popped_cond_.notify_all(); + } private: - size_type max_size_; - std::queue q_; - std::mutex mutex_; - std::condition_variable item_pushed_cond_; - std::condition_variable item_popped_cond_; - const std::chrono::hours one_hour{1}; + size_type max_size_; + std::queue q_; + std::mutex mutex_; + std::condition_variable item_pushed_cond_; + std::condition_variable item_popped_cond_; + const std::chrono::hours one_hour { + 1 + }; }; } diff --git a/include/c11log/details/fast_oss.h b/include/c11log/details/fast_oss.h index ca244e69..8ae803d5 100644 --- a/include/c11log/details/fast_oss.h +++ b/include/c11log/details/fast_oss.h @@ -53,7 +53,7 @@ public: fast_oss():std::ostream(&_dev) {} ~fast_oss() = default; fast_oss(const fast_oss& other):std::basic_ios(), std::ostream(),_dev(other._dev) {} - fast_oss operator=(const fast_oss& other) + fast_oss& operator=(const fast_oss& other) { if(&other != this) _dev = other._dev; diff --git a/include/c11log/details/line_logger.h b/include/c11log/details/line_logger.h index 6614a0b2..2ac5b601 100644 --- a/include/c11log/details/line_logger.h +++ b/include/c11log/details/line_logger.h @@ -13,7 +13,10 @@ namespace details class line_logger { public: - line_logger(logger* callback_logger, level::level_enum msg_level) { + line_logger(logger* callback_logger, level::level_enum msg_level): + _callback_logger(callback_logger), + _oss(), + _level(msg_level) { callback_logger->formatter_->format_header(callback_logger->logger_name_, msg_level, c11log::formatters::clock::now(), @@ -24,7 +27,9 @@ public: _callback_logger(other._callback_logger), _oss(other._oss), _level(other._level) {}; + line_logger& operator=(const line_logger&) = delete; + ~line_logger() { if (_callback_logger) { _oss << '\n'; diff --git a/include/c11log/logger.h b/include/c11log/logger.h index ff3a660f..689421a4 100644 --- a/include/c11log/logger.h +++ b/include/c11log/logger.h @@ -26,10 +26,13 @@ public: typedef std::shared_ptr sink_ptr_t; typedef std::vector sinks_vector_t; - explicit logger(const std::string& name) : logger_name_(name), - formatter_(std::make_unique()) { - atomic_level_.store(level::INFO); - } + explicit logger(const std::string& name) : + logger_name_(name), + formatter_(std::make_unique()), + sinks_(), + mutex_(), + atomic_level_(level::INFO) + {} ~logger() = default; @@ -55,7 +58,6 @@ public: private: friend details::line_logger; - std::string logger_name_ = ""; std::unique_ptr formatter_; sinks_vector_t sinks_; diff --git a/include/c11log/sinks/async_sink.h b/include/c11log/sinks/async_sink.h index 773f77d3..2529046e 100644 --- a/include/c11log/sinks/async_sink.h +++ b/include/c11log/sinks/async_sink.h @@ -31,7 +31,7 @@ protected: private: c11log::logger::sinks_vector_t sinks_; - std::atomic active_ { true }; + std::atomic active_; c11log::details::blocking_queue q_; std::thread back_thread_; //Clear all remaining messages(if any), stop the back_thread_ and join it @@ -45,7 +45,9 @@ private: /////////////////////////////////////////////////////////////////////////////// inline c11log::sinks::async_sink::async_sink(const std::size_t max_queue_size) - :q_(max_queue_size), + :sinks_(), + active_(true), + q_(max_queue_size), back_thread_(&async_sink::thread_loop_, this) {} diff --git a/include/c11log/sinks/file_sinks.h b/include/c11log/sinks/file_sinks.h index 063d6487..00d2a8c9 100644 --- a/include/c11log/sinks/file_sinks.h +++ b/include/c11log/sinks/file_sinks.h @@ -6,29 +6,29 @@ #include "base_sink.h" -namespace c11log { -namespace sinks { +namespace c11log +{ +namespace sinks +{ /* * Trivial file sink with single file as target */ -class simple_file_sink : public base_sink { +class simple_file_sink : public base_sink +{ public: - simple_file_sink(const std::string &filename, const std::string& extension = "txt") - { - std::ostringstream oss; - oss << filename << "." << extension; - _ofstream.open(oss.str(), std::ofstream::app); - } + explicit simple_file_sink(const std::string &filename, const std::string& extension = "txt") + : mutex_(), + _ofstream(filename + "." + extension, std::ofstream::app) + {} protected: - void sink_it_(const std::string& msg) override - { - std::lock_guard lock(mutex_); - _ofstream << msg; - _ofstream.flush(); - } + void sink_it_(const std::string& msg) override { + std::lock_guard lock(mutex_); + _ofstream << msg; + _ofstream.flush(); + } private: - std::mutex mutex_; - std::ofstream _ofstream; + std::mutex mutex_; + std::ofstream _ofstream; }; @@ -36,130 +36,124 @@ private: /* * Thread safe, size limited file sink */ -class rotating_file_sink : public base_sink { +class rotating_file_sink : public base_sink +{ public: - rotating_file_sink(const std::string &base_filename, const std::string &extension, size_t max_size, size_t max_files): - _base_filename(base_filename), - _extension(extension), - _max_size(max_size), - _max_files(max_files), - _current_size(0), - _index(0) - { - _ofstream.open(_calc_filename(_base_filename, 0, _extension)); - } + rotating_file_sink(const std::string &base_filename, const std::string &extension, size_t max_size, size_t max_files): + _base_filename(base_filename), + _extension(extension), + _max_size(max_size), + _max_files(max_files), + _current_size(0), + _index(0), + mutex_(), + _ofstream(_calc_filename(_base_filename, 0, _extension)) + {} protected: - void sink_it_(const std::string& msg) override - { - std::lock_guard lock(mutex_); - _current_size += msg.length(); - if (_current_size > _max_size) - { - _rotate(); - _current_size = msg.length(); - } - _ofstream << msg; - _ofstream.flush(); - } + void sink_it_(const std::string& msg) override { + std::lock_guard lock(mutex_); + _current_size += msg.length(); + if (_current_size > _max_size) { + _rotate(); + _current_size = msg.length(); + } + _ofstream << msg; + _ofstream.flush(); + } private: - static std::string _calc_filename(const std::string& filename, std::size_t index, const std::string& extension) - { - std::ostringstream oss; - if (index) - oss << filename << "." << index << "." << extension; - else - oss << filename << "." << extension; - return oss.str(); - } + static std::string _calc_filename(const std::string& filename, std::size_t index, const std::string& extension) { + std::ostringstream oss; + if (index) + oss << filename << "." << index << "." << extension; + else + oss << filename << "." << extension; + return oss.str(); + } - // Rotate old files: - // log.n-1.txt -> log.n.txt - // log n-2.txt -> log.n-1.txt - // ... - // log.txt -> log.1.txt - void _rotate() - { - _ofstream.close(); - //Remove oldest file - for (auto i = _max_files; i > 0; --i) { - auto src = _calc_filename(_base_filename, i - 1, _extension); - auto target = _calc_filename(_base_filename, i, _extension); - if (i == _max_files) - std::remove(target.c_str()); - std::rename(src.c_str(), target.c_str()); - } - _ofstream.open(_calc_filename(_base_filename, 0, _extension)); - } + // Rotate old files: + // log.n-1.txt -> log.n.txt + // log n-2.txt -> log.n-1.txt + // ... + // log.txt -> log.1.txt + void _rotate() { + _ofstream.close(); + //Remove oldest file + for (auto i = _max_files; i > 0; --i) { + auto src = _calc_filename(_base_filename, i - 1, _extension); + auto target = _calc_filename(_base_filename, i, _extension); + if (i == _max_files) + std::remove(target.c_str()); + std::rename(src.c_str(), target.c_str()); + } + _ofstream.open(_calc_filename(_base_filename, 0, _extension)); + } - std::string _base_filename; - std::string _extension; - std::size_t _max_size; - std::size_t _max_files; - std::size_t _current_size; - std::size_t _index; - std::ofstream _ofstream; - std::mutex mutex_; + std::string _base_filename; + std::string _extension; + std::size_t _max_size; + std::size_t _max_files; + std::size_t _current_size; + std::size_t _index; + std::mutex mutex_; + std::ofstream _ofstream; }; /* * Thread safe file sink that closes the log file at midnight and opens new one */ -class daily_file_sink:public base_sink { +class daily_file_sink:public base_sink +{ public: - daily_file_sink(const std::string& base_filename, const std::string& extension = "txt"): - _base_filename(base_filename), - _extension(extension), - _midnight_tp { _calc_midnight_tp() } - - { - _ofstream.open(_calc_filename(_base_filename, _extension), std::ofstream::app); - } + explicit daily_file_sink(const std::string& base_filename, const std::string& extension = "txt"): + _base_filename(base_filename), + _extension(extension), + _midnight_tp (_calc_midnight_tp() ), + mutex_(), + _ofstream(_calc_filename(_base_filename, _extension), std::ofstream::app) + {} protected: - void sink_it_(const std::string& msg) override - { - std::lock_guard lock(mutex_); - if (std::chrono::system_clock::now() >= _midnight_tp) - { - _ofstream.close(); - _ofstream.open(_calc_filename(_base_filename, _extension)); - _midnight_tp = _calc_midnight_tp(); - } - _ofstream << msg; - _ofstream.flush(); - } + void sink_it_(const std::string& msg) override { + std::lock_guard lock(mutex_); + if (std::chrono::system_clock::now() >= _midnight_tp) { + _ofstream.close(); + _ofstream.open(_calc_filename(_base_filename, _extension)); + _midnight_tp = _calc_midnight_tp(); + } + _ofstream << msg; + _ofstream.flush(); + } private: - // Return next midnight's time_point - static std::chrono::system_clock::time_point _calc_midnight_tp() - { - using namespace std::chrono; - auto now = system_clock::now(); - time_t tnow = std::chrono::system_clock::to_time_t(now); - tm date = c11log::details::os::localtime(tnow); - date.tm_hour = date.tm_min = date.tm_sec = 0; - auto midnight = std::chrono::system_clock::from_time_t(std::mktime(&date)); - return system_clock::time_point(midnight + hours(24)); - } + // Return next midnight's time_point + static std::chrono::system_clock::time_point _calc_midnight_tp() { + using namespace std::chrono; + auto now = system_clock::now(); + time_t tnow = std::chrono::system_clock::to_time_t(now); + tm date = c11log::details::os::localtime(tnow); + date.tm_hour = date.tm_min = date.tm_sec = 0; + auto midnight = std::chrono::system_clock::from_time_t(std::mktime(&date)); + return system_clock::time_point(midnight + hours(24)); + } - static std::string _calc_filename(const std::string& basename, const std::string& extension) - { - std::tm tm = c11log::details::os::localtime(); - char buf[32]; - sprintf(buf, ".%d-%02d-%02d.", tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday); - return basename+buf+extension; - } + static std::string _calc_filename(const std::string& basename, const std::string& extension) { + std::tm tm = c11log::details::os::localtime(); + char buf[32]; + sprintf(buf, ".%d-%02d-%02d.", tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday); + return basename+buf+extension; + } + + std::string _base_filename; + std::string _extension; + std::chrono::system_clock::time_point _midnight_tp; + std::mutex mutex_; + std::ofstream _ofstream; - std::string _base_filename; - std::string _extension; - std::chrono::system_clock::time_point _midnight_tp; - std::ofstream _ofstream; - std::mutex mutex_; }; } }