Linter fixes for audiocache.cc

5 files changed, 114 insertions, 84 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index eac6653..a8bdb59 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -122,6 +122,7 @@ EXTRA_DIST = \
 	midimapper.h \
 	nolocale.h \
 	notifier.h \
+	owner.h \
 	path.h \
 	platform.h \
 	powerlist.h \
diff --git a/src/audiocache.cc b/src/audiocache.cc
index a2d26d9..fa144fd 100644
--- a/src/audiocache.cc
+++ b/src/audiocache.cc
@@ -28,8 +28,8 @@
 
 #include <mutex>
 
-#include <stdio.h>
-#include <assert.h>
+#include <cstdio>
+#include <cassert>
 
 #include <hugin.hpp>
 
@@ -64,9 +64,9 @@ void AudioCache::deinit()
 }
 
 // Invariant: initial_samples_needed < preloaded audio data
-sample_t* AudioCache::open(const AudioFile& file,
-                           std::size_t initial_samples_needed,
-                           int channel, cacheid_t& id)
+gsl::owner<sample_t*> AudioCache::open(const AudioFile& file,
+                                       std::size_t initial_samples_needed,
+                                       int channel, cacheid_t& new_cacheid)
 {
 	assert(chunk_size); // Assert updateChunkSize was called before processing.
 
@@ -74,16 +74,16 @@ sample_t* AudioCache::open(const AudioFile& file,
 	{
 		settings.number_of_underruns.fetch_add(1);
 		// File preload not yet ready - skip this sample.
-		id = CACHE_DUMMYID;
+		new_cacheid = CACHE_DUMMYID;
 		assert(nodata);
 		return nodata;
 	}
 
 	// Register a new id for this cache session.
-	id = id_manager.registerID({});
+	new_cacheid = id_manager.registerID({});
 
 	// If we are out of available ids we get CACHE_DUMMYID
-	if(id == CACHE_DUMMYID)
+	if(new_cacheid == CACHE_DUMMYID)
 	{
 		settings.number_of_underruns.fetch_add(1);
 		// Use nodata buffer instead.
@@ -92,19 +92,19 @@ sample_t* AudioCache::open(const AudioFile& file,
 	}
 
 	// Get the cache_t connected with the registered id.
-	cache_t& c = id_manager.getCache(id);
+	cache_t& cache = id_manager.getCache(new_cacheid);
 
-	c.afile = nullptr; // File is opened when needed.
-	c.channel = channel;
+	cache.afile = nullptr; // File is opened when needed.
+	cache.channel = channel;
 
 	// Next call to 'next()' will read from this point.
-	c.localpos = initial_samples_needed;
+	cache.localpos = initial_samples_needed;
 
-	c.ready = false;
-	c.front = nullptr; // This is allocated when needed.
-	c.back = nullptr; // This is allocated when needed.
+	cache.ready = false;
+	cache.front = nullptr; // This is allocated when needed.
+	cache.back = nullptr; // This is allocated when needed.
 
-	std::size_t cropped_size;
+	std::size_t cropped_size{};
 
 	if(file.preloadedsize == file.size)
 	{
@@ -122,162 +122,162 @@ sample_t* AudioCache::open(const AudioFile& file,
 		//  \----------------------v-------------------/
 		//                     cropped_size
 
-		cropped_size = file.preloadedsize - c.localpos;
+		cropped_size = file.preloadedsize - cache.localpos;
 		cropped_size -= cropped_size % framesize;
 		cropped_size += initial_samples_needed;
 	}
 
-	c.preloaded_samples = file.data;
-	c.preloaded_samples_size = cropped_size;
+	cache.preloaded_samples = file.data;
+	cache.preloaded_samples_size = cropped_size;
 
 	// Next potential read from disk will read from this point.
-	c.pos = cropped_size;
+	cache.pos = cropped_size;
 
 	// Only load next buffer if there is more data in the file to be loaded...
-	if(c.pos < file.size)
+	if(cache.pos < file.size)
 	{
-		c.afile = &event_handler.openFile(file.filename);
+		cache.afile = &event_handler.openFile(file.filename);
 
-		if(c.back == nullptr)
+		if(cache.back == nullptr)
 		{
-			c.back = new sample_t[chunk_size];
+			cache.back = new sample_t[chunk_size];
 		}
 
-		event_handler.pushLoadNextEvent(c.afile, c.channel, c.pos,
-		                                c.back, &c.ready);
+		event_handler.pushLoadNextEvent(cache.afile, cache.channel, cache.pos,
+		                                cache.back, &cache.ready);
 	}
 
-	return c.preloaded_samples; // return preloaded data
+	return cache.preloaded_samples; // return preloaded data
 }
 
-sample_t* AudioCache::next(cacheid_t id, std::size_t& size)
+gsl::owner<sample_t*> AudioCache::next(cacheid_t cacheid, std::size_t& size)
 {
-	if(id == CACHE_DUMMYID)
+	if(cacheid == CACHE_DUMMYID)
 	{
 		settings.number_of_underruns.fetch_add(1);
 		assert(nodata);
 		return nodata;
 	}
 
-	cache_t& c = id_manager.getCache(id);
+	cache_t& cache = id_manager.getCache(cacheid);
 
-	if(c.preloaded_samples)
+	if(cache.preloaded_samples != nullptr)
 	{
 		// We are playing from memory:
-		if(c.localpos < c.preloaded_samples_size)
+		if(cache.localpos < cache.preloaded_samples_size)
 		{
-			sample_t* s = c.preloaded_samples + c.localpos;
+			sample_t* samples = cache.preloaded_samples + cache.localpos; // NOLINT: Fix with span?
 			// If only a partial frame is returned. Reflect this in the size
-			size = std::min(size, c.preloaded_samples_size - c.localpos);
+			size = std::min(size, cache.preloaded_samples_size - cache.localpos);
 
-			c.localpos += size;
-			return s;
+			cache.localpos += size;
+			return samples;
 		}
 
-		c.preloaded_samples = nullptr; // Start using samples from disk.
+		cache.preloaded_samples = nullptr; // Start using samples from disk.
 
 	}
 	else
 	{
 		// We are playing from cache:
-		if(c.localpos < chunk_size)
+		if(cache.localpos < chunk_size)
 		{
-			if(c.front == nullptr)
+			if(cache.front == nullptr)
 			{
 				// Just return silence.
 				settings.number_of_underruns.fetch_add(1);
-				c.localpos += size; // Skip these samples so we don't loose sync.
+				cache.localpos += size; // Skip these samples so we don't loose sync.
 				assert(nodata);
 				return nodata;
 			}
 
-			sample_t* s = c.front + c.localpos;
+			sample_t* samples = cache.front + cache.localpos;// NOLINT: Fix with span?
 			// If only a partial frame is returned. Reflect this in the size
-			size = std::min(size, chunk_size - c.localpos);
-			c.localpos += size;
-			return s;
+			size = std::min(size, chunk_size - cache.localpos);
+			cache.localpos += size;
+			return samples;
 		}
 	}
 
 	// Check for buffer underrun
-	if(!c.ready)
+	if(!cache.ready)
 	{
 		// Just return silence.
 		settings.number_of_underruns.fetch_add(1);
-		c.localpos += size; // Skip these samples so we don't loose sync.
+		cache.localpos += size; // Skip these samples so we don't loose sync.
 		assert(nodata);
 		return nodata;
 	}
 
 	// Swap buffers
-	std::swap(c.front, c.back);
+	std::swap(cache.front, cache.back);
 
 	// Next time we go here we have already read the first frame.
-	c.localpos = size;
+	cache.localpos = size;
 
-	c.pos += chunk_size;
+	cache.pos += chunk_size;
 
 	// Does the file have remaining unread samples?
-	assert(c.afile); // Assert that we have an audio file.
-	if(c.pos < c.afile->getSize())
+	assert(cache.afile); // Assert that we have an audio file.
+	if(cache.pos < cache.afile->getSize())
 	{
 		// Do we have a back buffer to read into?
-		if(c.back == nullptr)
+		if(cache.back == nullptr)
 		{
-			c.back = new sample_t[chunk_size];
+			cache.back = new sample_t[chunk_size];
 		}
 
-		event_handler.pushLoadNextEvent(c.afile, c.channel, c.pos,
-		                                c.back, &c.ready);
+		event_handler.pushLoadNextEvent(cache.afile, cache.channel, cache.pos,
+		                                cache.back, &cache.ready);
 	}
 
 	// We should always have a front buffer at this point.
-	assert(c.front);
-	return c.front;
+	assert(cache.front);
+	return cache.front;
 }
 
-bool AudioCache::isReady(cacheid_t id)
+bool AudioCache::isReady(cacheid_t cacheid)
 {
-	if(id == CACHE_DUMMYID)
+	if(cacheid == CACHE_DUMMYID)
 	{
 		return true;
 	}
 
-	cache_t& cache = id_manager.getCache(id);
+	const cache_t& cache = id_manager.getCache(cacheid);
 	return cache.ready;
 }
 
-void AudioCache::close(cacheid_t id)
+void AudioCache::close(cacheid_t cacheid)
 {
-	if(id == CACHE_DUMMYID)
+	if(cacheid == CACHE_DUMMYID)
 	{
 		return;
 	}
 
-	event_handler.pushCloseEvent(id);
+	event_handler.pushCloseEvent(cacheid);
 }
 
 void AudioCache::setFrameSize(std::size_t framesize)
 {
 	// Make sure the event handler thread is stalled while we set the framesize
 	// state.
-	std::lock_guard<AudioCacheEventHandler> event_handler_lock(event_handler);
+	const std::lock_guard<AudioCacheEventHandler> event_handler_lock(event_handler);
 
 	// NOTE: Not threaded...
 	//std::lock_guard<AudioCacheIDManager> id_manager_lock(id_manager);
 
 	if(framesize > nodata_framesize)
 	{
-		if(nodata)
+		if(nodata != nullptr)
 		{
-			nodata_dirty.emplace_back(std::move(nodata)); // Store for later deletion.
+			nodata_dirty.emplace_back(nodata); // Store for later deletion.
 		}
 		nodata = new sample_t[framesize];
 		nodata_framesize = framesize;
 
 		for(std::size_t i = 0; i < framesize; ++i)
 		{
-			nodata[i] = 0.0f;
+			nodata[i] = 0.0f;// NOLINT: Fix with span?
 		}
 	}
 
@@ -292,9 +292,11 @@ std::size_t AudioCache::getFrameSize() const
 void AudioCache::updateChunkSize(std::size_t output_channels)
 {
 	// Make sure we won't get out-of-range chunk sizes.
-	std::size_t disk_cache_chunk_size =
-		std::max(settings.disk_cache_chunk_size.load(), std::size_t(512u * 1024u));
-	output_channels = std::max(output_channels, std::size_t(1u));
+	constexpr std::size_t max_cache_chunk_size{512ul * 1024ul};
+	const auto disk_cache_chunk_size =
+		std::max(settings.disk_cache_chunk_size.load(), max_cache_chunk_size);
+	constexpr std::size_t min_output_channels{1};
+	output_channels = std::max(output_channels, min_output_channels);
 
 	// 1MB pr. chunk divided over 16 channels, 4 bytes pr. sample.
 	const auto ideal_chunk_size =
diff --git a/src/audiocache.h b/src/audiocache.h
index 9c6fa53..632aae9 100644
--- a/src/audiocache.h
+++ b/src/audiocache.h
@@ -38,6 +38,7 @@
 #include "audiocacheidmanager.h"
 #include "audiocacheeventhandler.h"
 #include "settings.h"
+#include "owner.h"
 
 class AudioCache
 {
@@ -68,8 +69,10 @@ public:
 	//! \param [out] new_id The newly created cache id.
 	//! \return A pointer to the first buffer containing the
 	//!  'initial_samples_needed' number of samples.
-	sample_t* open(const AudioFile& file, std::size_t initial_samples_needed, int channel,
-	               cacheid_t& new_id);
+	gsl::owner<sample_t*> open(const AudioFile& file,
+	                           std::size_t initial_samples_needed,
+	                           int channel,
+	                           cacheid_t& new_cacheid);
 
 	//! Get next buffer.
 	//! Returns the next buffer for reading based on cache id.
@@ -78,9 +81,9 @@ public:
 	//! \param id The cache id to read from.
 	//! \param [out] size The size of the returned buffer.
 	//! \return A pointer to the buffer.
-	sample_t* next(cacheid_t id, std::size_t &size);
+	gsl::owner<sample_t*> next(cacheid_t id, std::size_t &size);
 
-	//! Returns true iff the next chunk of the supplied id has been read from disk.
+	//! Returns true iff next chunk of the supplied id has been read from disk.
 	bool isReady(cacheid_t id);
 
 	//! Unregister cache entry.
@@ -103,10 +106,10 @@ public:
 	bool isAsyncMode() const;
 
 private:
-	std::size_t framesize{0};
-	sample_t* nodata{nullptr};
-	std::size_t nodata_framesize{0};
-	std::size_t chunk_size{0};
+	std::size_t framesize{};
+	gsl::owner<sample_t*> nodata{};
+	std::size_t nodata_framesize{};
+	std::size_t chunk_size{};
 	std::list<std::unique_ptr<sample_t[]>> nodata_dirty;
 
 	AudioCacheIDManager id_manager;
diff --git a/src/audiocacheidmanager.h b/src/audiocacheidmanager.h
index 0aa40e3..2bbe09d 100644
--- a/src/audiocacheidmanager.h
+++ b/src/audiocacheidmanager.h
@@ -26,14 +26,14 @@
  */
 #pragma once
 
-#include <stdlib.h>
+#include <cstdlib>
 
 #include <vector>
-
-#include <audiotypes.h>
-
 #include <mutex>
 
+#include "audiotypes.h"
+#include "owner.h"
+
 class AudioCacheFile;
 
 #define CACHE_DUMMYID -2
@@ -49,8 +49,8 @@ struct cache_t
 	size_t channel{0};
 	size_t pos{0}; //< File position
 	volatile bool ready{false};
-	sample_t* front{nullptr};
-	sample_t* back{nullptr};
+	gsl::owner<sample_t*> front{nullptr};
+	gsl::owner<sample_t*> back{nullptr};
 	size_t localpos{0}; //< Intra buffer (front) position.
 
 	sample_t* preloaded_samples{nullptr}; // nullptr means preload buffer not active.
diff --git a/src/owner.h b/src/owner.h
new file mode 100644
index 0000000..0c67ae5
--- /dev/null
+++ b/src/owner.h
@@ -0,0 +1,24 @@
+//! -*- c++ -*-
+#pragma once
+
+//#include <type_traits>  // for enable_if_t, is_convertible, is_assignable
+
+// Taken from the GSL Core Guidelines Support Library
+namespace gsl
+{
+
+//
+// owner
+//
+// `gsl::owner<T>` is designed as a safety mechanism for code that must deal directly with raw pointers that own memory.
+// Ideally such code should be restricted to the implementation of low-level abstractions. `gsl::owner` can also be used
+// as a stepping point in converting legacy code to use more modern RAII constructs, such as smart pointers.
+//
+// T must be a pointer type
+// - disallow construction from any type other than pointer type
+//
+//template <class T, class = std::enable_if_t<std::is_pointer<T>::value>>
+template<typename T>
+using owner = T;
+
+}// gsl::