From 8b5a32564494b7838ea36a66d45625f2ba5aebd5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Beat=20K=C3=BCng?= <beat-kueng@gmx.net>
Date: Mon, 2 May 2016 15:07:15 +0200
Subject: [PATCH] logger: remove _log_buffer from Logger, initialize it in the
 writer instead

it's not used in the logger, so don't store it there. It is accessed via
LogWriter::write.
This also makes sure the buffer size is >= _min_write_chunk and handles
allocation failure properly.
---
 src/modules/logger/log_writer.cpp | 26 ++++++++++++++++++++++----
 src/modules/logger/log_writer.h   | 15 ++++++++++-----
 src/modules/logger/logger.cpp     | 10 ++++++----
 src/modules/logger/logger.h       |  3 +--
 4 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/modules/logger/log_writer.cpp b/src/modules/logger/log_writer.cpp
index 8a75dc9102..cb1c321a07 100644
--- a/src/modules/logger/log_writer.cpp
+++ b/src/modules/logger/log_writer.cpp
@@ -2,15 +2,15 @@
 #include <fcntl.h>
 #include <string.h>
 
+#include <mathlib/mathlib.h>
+
 namespace px4
 {
 namespace logger
 {
 
-LogWriter::LogWriter(uint8_t *buffer, size_t buffer_size) :
-	_buffer(buffer),
-	_buffer_size(buffer_size),
-	_min_write_chunk(4096)
+LogWriter::LogWriter(size_t buffer_size) :
+	_buffer_size(math::max(buffer_size, _min_write_chunk))
 {
 	pthread_mutex_init(&_mtx, nullptr);
 	pthread_cond_init(&_cv, nullptr);
@@ -19,6 +19,24 @@ LogWriter::LogWriter(uint8_t *buffer, size_t buffer_size) :
 	perf_fsync = perf_alloc(PC_ELAPSED, "sd fsync");
 }
 
+bool LogWriter::init()
+{
+	if (_buffer) {
+		return true;
+	}
+
+	_buffer = new uint8_t[_buffer_size];
+
+	return _buffer;
+}
+
+LogWriter::~LogWriter()
+{
+	if (_buffer) {
+		delete[] _buffer;
+	}
+}
+
 void LogWriter::start_log(const char *filename)
 {
 	::strncpy(_filename, filename, sizeof(_filename));
diff --git a/src/modules/logger/log_writer.h b/src/modules/logger/log_writer.h
index e49ab4efe4..2aef0344f2 100644
--- a/src/modules/logger/log_writer.h
+++ b/src/modules/logger/log_writer.h
@@ -15,7 +15,10 @@ class LogWriter
 {
 	friend class Logger;
 public:
-	LogWriter(uint8_t *buffer, size_t buffer_size);
+	LogWriter(size_t buffer_size);
+	~LogWriter();
+
+	bool init();
 
 	/**
 	 * start the thread
@@ -59,13 +62,15 @@ private:
 		_count -= n;
 	}
 
+	/* 512 didn't seem to work properly, 4096 should match the FAT cluster size */
+	static const size_t	_min_write_chunk = 4096;
+
 	char		_filename[64];
 	int			_fd;
-	uint8_t 	*_buffer;
+	uint8_t 	*_buffer = nullptr;
 	const size_t	_buffer_size;
-	const size_t	_min_write_chunk;	/* 512 didn't seem to work properly, 4096 should match the FAT cluster size */
-	size_t			_head = 0;
-	size_t			_count = 0;
+	size_t			_head = 0; ///< next position to write to
+	size_t			_count = 0; ///< number of bytes in _buffer to be written
 	size_t		_total_written = 0;
 	bool		_should_run = false;
 	bool		_running = false;
diff --git a/src/modules/logger/logger.cpp b/src/modules/logger/logger.cpp
index 9d547adf52..dd23e1521b 100644
--- a/src/modules/logger/logger.cpp
+++ b/src/modules/logger/logger.cpp
@@ -254,7 +254,7 @@ static constexpr size_t MAX_DATA_SIZE = 740;
 
 Logger::Logger(size_t buffer_size, unsigned log_interval, bool log_on_start) :
 	_log_on_start(log_on_start),
-	_writer((_log_buffer = new uint8_t[buffer_size]), buffer_size),
+	_writer(buffer_size),
 	_log_interval(log_interval)
 {
 }
@@ -280,9 +280,6 @@ Logger::~Logger()
 			}
 		} while (logger_task != -1);
 	}
-
-	delete [] _log_buffer;
-	logger_ptr = nullptr;
 }
 
 int Logger::add_topic(const orb_metadata *topic)
@@ -414,6 +411,11 @@ void Logger::run()
 //	add_topic("estimator_status");
 	add_topic("vehicle_status", 100);
 
+	if (!_writer.init()) {
+		PX4_ERR("logger: init of writer failed");
+		return;
+	}
+
 	int ret = _writer.thread_start(_writer_thread);
 
 	if (ret) {
diff --git a/src/modules/logger/logger.h b/src/modules/logger/logger.h
index f19de0ccb7..c25e7171e6 100644
--- a/src/modules/logger/logger.h
+++ b/src/modules/logger/logger.h
@@ -80,13 +80,12 @@ private:
 
 	bool copy_if_updated_multi(orb_id_t topic, int multi_instance, int *handle, void *buffer, uint64_t *time_last_checked);
 
-	static constexpr size_t 	MAX_TOPICS_NUM = 128;
+	static constexpr size_t 	MAX_TOPICS_NUM = 128; /**< Maximum number of logged topics */
 	static constexpr unsigned	MAX_NO_LOGFOLDER = 999;	/**< Maximum number of log dirs */
 	static constexpr unsigned	MAX_NO_LOGFILE = 999;	/**< Maximum number of log files */
 	static constexpr const char 		*LOG_ROOT = PX4_ROOTFSDIR"/fs/microsd/log";
 
 	bool						_task_should_exit = true;
-	uint8_t 					*_log_buffer;
 	char 						_log_dir[64];
 	uORB::Subscription<vehicle_status_s>	_vehicle_status_sub {ORB_ID(vehicle_status)};
 	uORB::Subscription<parameter_update_s>	_parameter_update_sub {ORB_ID(parameter_update)};