db/update/InotifyUpdate: convert to class, no global variables

src/Instance.cxx

@@ -37,6 +37,10 @@
 #include "db/update/Service.hxx"
 #include "storage/StorageInterface.hxx"
 
+#ifdef ENABLE_INOTIFY
+#include "db/update/InotifyUpdate.hxx"
+#endif
+
 #ifdef ENABLE_NEIGHBOR_PLUGINS
 #include "neighbor/Glue.hxx"
 #endif

src/Instance.hxx

@@ -43,6 +43,9 @@ class NeighborGlue;
 #include "db/Ptr.hxx"
 class Storage;
 class UpdateService;
+#ifdef ENABLE_INOTIFY
+class InotifyUpdate;
+#endif
 #endif
 
 #include <memory>
@@ -120,6 +123,10 @@ struct Instance final
 	Storage *storage = nullptr;
 
 	UpdateService *update = nullptr;
+
+#ifdef ENABLE_INOTIFY
+	std::unique_ptr<InotifyUpdate> inotify_update;
+#endif
 #endif
 
 #ifdef ENABLE_CURL

src/Main.cxx

@@ -344,7 +344,7 @@ Instance::BeginShutdownUpdate() noexcept
 {
 #ifdef ENABLE_DATABASE
 #ifdef ENABLE_INOTIFY
-	mpd_inotify_finish();
+	inotify_update.reset();
 #endif
 
 	if (update != nullptr)
@@ -524,6 +524,7 @@ MainConfigured(const struct options &options, const ConfigData &raw_config)
 #ifdef ENABLE_INOTIFY
 		if (instance.storage != nullptr &&
 		    instance.update != nullptr)
+			instance.inotify_update =
 				mpd_inotify_init(instance.event_loop,
 						 *instance.storage,
 						 *instance.update,

src/db/update/InotifyUpdate.cxx

@@ -18,8 +18,6 @@
  */
 
 #include "InotifyUpdate.hxx"
-#include "InotifySource.hxx"
-#include "InotifyQueue.hxx"
 #include "InotifyDomain.hxx"
 #include "ExcludeList.hxx"
 #include "lib/fmt/ExceptionFormatter.hxx"
@@ -38,7 +36,6 @@
 #include <cassert>
 #include <cstring>
 #include <forward_list>
-#include <map>
 #include <string>
 
 #include <sys/inotify.h>
@@ -99,65 +96,46 @@ try {
 		LogError(std::current_exception());
 }
 
-static InotifySource *inotify_source;
+void
-static InotifyQueue *inotify_queue;
+InotifyUpdate::AddToMap(WatchDirectory &directory) noexcept
-
-static unsigned inotify_max_depth;
-static WatchDirectory *inotify_root;
-static std::map<int, WatchDirectory *> inotify_directories;
-
-static void
-tree_add_watch_directory(WatchDirectory *directory)
 {
-	inotify_directories.emplace(directory->descriptor, directory);
+	directories.emplace(directory.descriptor, &directory);
 }
 
-static void
+void
-tree_remove_watch_directory(WatchDirectory *directory)
+InotifyUpdate::RemoveFromMap(WatchDirectory &directory) noexcept
 {
-	auto i = inotify_directories.find(directory->descriptor);
+	auto i = directories.find(directory.descriptor);
-	assert(i != inotify_directories.end());
+	assert(i != directories.end());
-	inotify_directories.erase(i);
+	directories.erase(i);
 }
 
-static WatchDirectory *
+void
-tree_find_watch_directory(int wd)
+InotifyUpdate::Disable(WatchDirectory &directory) noexcept
 {
-	auto i = inotify_directories.find(wd);
+	RemoveFromMap(directory);
-	if (i == inotify_directories.end())
-		return nullptr;
-
-	return i->second;
-}
-
-static void
-disable_watch_directory(WatchDirectory &directory)
-{
-	tree_remove_watch_directory(&directory);
 
 	for (WatchDirectory &child : directory.children)
-		disable_watch_directory(child);
+		Disable(child);
 
-	inotify_source->Remove(directory.descriptor);
+	source.Remove(directory.descriptor);
 }
 
-static void
+void
-remove_watch_directory(WatchDirectory *directory)
+InotifyUpdate::Delete(WatchDirectory &directory) noexcept
 {
-	assert(directory != nullptr);
+	if (directory.parent == nullptr) {
-
-	if (directory->parent == nullptr) {
 		LogWarning(inotify_domain,
 			   "music directory was removed - "
 			   "cannot continue to watch it");
 		return;
 	}
 
-	disable_watch_directory(*directory);
+	Disable(directory);
 
 	/* remove it from the parent, which effectively deletes it */
-	directory->parent->children.remove_if([directory](const WatchDirectory &child){
+	directory.parent->children.remove_if([&directory](const WatchDirectory &child){
-			return &child == directory;
+		return &child == &directory;
 	});
 }
 
@@ -183,17 +161,17 @@ SkipFilename(Path name) noexcept
 		name.HasNewline();
 }
 
-static void
+void
-recursive_watch_subdirectories(WatchDirectory &parent,
+InotifyUpdate::RecursiveWatchSubdirectories(WatchDirectory &parent,
 					    const Path path_fs,
-			       unsigned depth)
+					    unsigned depth) noexcept
 try {
-	assert(depth <= inotify_max_depth);
+	assert(depth <= max_depth);
 	assert(!path_fs.IsNull());
 
 	++depth;
 
-	if (depth > inotify_max_depth)
+	if (depth > max_depth)
 		return;
 
 	DirectoryReader dir(path_fs);
@@ -221,29 +199,27 @@ try {
 			continue;
 
 		try {
-			ret = inotify_source->Add(child_path_fs.c_str(),
+			ret = source.Add(child_path_fs.c_str(), IN_MASK);
-						  IN_MASK);
 		} catch (...) {
 			FmtError(inotify_domain,
-				 "Failed to register %s: {}",
+				 "Failed to register {}: {}",
 				 child_path_fs, std::current_exception());
 			continue;
 		}
 
-		WatchDirectory *child = tree_find_watch_directory(ret);
+		if (directories.find(ret) != directories.end())
-		if (child != nullptr)
 			/* already being watched */
 			continue;
 
 		parent.children.emplace_front(parent,
 					      name_fs,
 					      ret);
-		child = &parent.children.front();
+		auto *child = &parent.children.front();
 		child->LoadExcludeList(child_path_fs);
 
-		tree_add_watch_directory(child);
+		AddToMap(*child);
 
-		recursive_watch_subdirectories(*child, child_path_fs, depth);
+		RecursiveWatchSubdirectories(*child, child_path_fs, depth);
 	}
 } catch (...) {
 	LogError(std::current_exception());
@@ -261,20 +237,44 @@ WatchDirectory::GetDepth() const noexcept
 	return depth;
 }
 
-static void
+inline
-mpd_inotify_callback(int wd, unsigned mask,
+InotifyUpdate::InotifyUpdate(EventLoop &loop, UpdateService &update,
-		     [[maybe_unused]] const char *name, [[maybe_unused]] void *ctx)
+			     unsigned _max_depth)
+	:source(loop, InotifyCallback, this),
+	 queue(loop, update),
+	 max_depth(_max_depth)
 {
-	WatchDirectory *directory;
+}
 
-	directory = tree_find_watch_directory(wd);
+InotifyUpdate::~InotifyUpdate() noexcept = default;
-	if (directory == nullptr)
+
+inline void
+InotifyUpdate::Start(Path path)
+{
+	int descriptor = source.Add(path.c_str(), IN_MASK);
+
+	root = std::make_unique<WatchDirectory>(path, descriptor);
+	root->LoadExcludeList(path);
+
+	AddToMap(*root);
+
+	RecursiveWatchSubdirectories(*root, path, 0);
+}
+
+void
+InotifyUpdate::InotifyCallback(int wd, unsigned mask,
+			       [[maybe_unused]] const char *name) noexcept
+{
+	auto i = directories.find(wd);
+	if (i == directories.end())
 		return;
 
-	const auto uri_fs = directory->GetUriFS();
+	auto &directory = *i->second;
+
+	const auto uri_fs = directory.GetUriFS();
 
 	if ((mask & (IN_DELETE_SELF|IN_MOVE_SELF)) != 0) {
-		remove_watch_directory(directory);
+		Delete(directory);
 		return;
 	}
 
@@ -282,20 +282,20 @@ mpd_inotify_callback(int wd, unsigned mask,
 	    (mask & IN_ISDIR) != 0) {
 		/* a sub directory was changed: register those in
 		   inotify */
-		const auto &root = inotify_root->name;
+		const Path root_path = root->name;
 
 		const auto path_fs = uri_fs.IsNull()
-			? root
+			? root_path
-			: (root / uri_fs);
+			: (root_path / uri_fs);
 
-		recursive_watch_subdirectories(*directory, path_fs,
+		RecursiveWatchSubdirectories(directory, path_fs,
-					       directory->GetDepth());
+					     directory.GetDepth());
 	}
 
 	if ((mask & (IN_CLOSE_WRITE|IN_MOVE|IN_DELETE)) != 0 ||
 	    /* at the maximum depth, we watch out for newly created
 	       directories */
-	    (directory->GetDepth() == inotify_max_depth &&
+	    (directory.GetDepth() == max_depth &&
 	     (mask & (IN_CREATE|IN_ISDIR)) == (IN_CREATE|IN_ISDIR))) {
 		/* a file was changed, or a directory was
 		   moved/deleted: queue a database update */
@@ -303,14 +303,14 @@ mpd_inotify_callback(int wd, unsigned mask,
 		if (!uri_fs.IsNull()) {
 			const std::string uri_utf8 = uri_fs.ToUTF8();
 			if (!uri_utf8.empty())
-				inotify_queue->Enqueue(uri_utf8.c_str());
+				queue.Enqueue(uri_utf8.c_str());
 		}
 		else
-			inotify_queue->Enqueue("");
+			queue.Enqueue("");
 	}
 }
 
-void
+std::unique_ptr<InotifyUpdate>
 mpd_inotify_init(EventLoop &loop, Storage &storage, UpdateService &update,
 		 unsigned max_depth)
 {
@@ -319,50 +319,13 @@ mpd_inotify_init(EventLoop &loop, Storage &storage, UpdateService &update,
 	const auto path = storage.MapFS("");
 	if (path.IsNull()) {
 		LogDebug(inotify_domain, "no music directory configured");
-		return;
+		return {};
 	}
 
-	try {
+	auto iu = std::make_unique<InotifyUpdate>(loop, update, max_depth);
-		inotify_source = new InotifySource(loop,
+	iu->Start(path);
-						   mpd_inotify_callback,
-						   nullptr);
-	} catch (...) {
-		LogError(std::current_exception());
-		return;
-	}
-
-	inotify_max_depth = max_depth;
-
-	int descriptor;
-	try {
-		descriptor = inotify_source->Add(path.c_str(), IN_MASK);
-	} catch (...) {
-		LogError(std::current_exception());
-		delete inotify_source;
-		inotify_source = nullptr;
-		return;
-	}
-
-	inotify_root = new WatchDirectory(path, descriptor);
-	inotify_root->LoadExcludeList(path);
-
-	tree_add_watch_directory(inotify_root);
-
-	recursive_watch_subdirectories(*inotify_root, path, 0);
-
-	inotify_queue = new InotifyQueue(loop, update);
 
 	LogDebug(inotify_domain, "watching music directory");
-}
 
-void
+	return iu;
-mpd_inotify_finish() noexcept
-{
-	if (inotify_source == nullptr)
-		return;
-
-	delete inotify_queue;
-	delete inotify_source;
-	delete inotify_root;
-	inotify_directories.clear();
 }

src/db/update/InotifyUpdate.hxx

@@ -20,15 +20,59 @@
 #ifndef MPD_INOTIFY_UPDATE_HXX
 #define MPD_INOTIFY_UPDATE_HXX
 
-class EventLoop;
+#include "InotifySource.hxx"
-class Storage;
+#include "InotifyQueue.hxx"
-class UpdateService;
 
-void
+#include <map>
+#include <memory>
+
+class Path;
+class Storage;
+struct WatchDirectory;
+
+/**
+ * Glue code between InotifySource and InotifyQueue.
+ */
+class InotifyUpdate {
+	InotifySource source;
+	InotifyQueue queue;
+
+	const unsigned max_depth;
+
+	std::unique_ptr<WatchDirectory> root;
+	std::map<int, WatchDirectory *> directories;
+
+public:
+	InotifyUpdate(EventLoop &loop, UpdateService &update,
+		      unsigned _max_depth);
+	~InotifyUpdate() noexcept;
+
+	void Start(Path path);
+
+private:
+	void InotifyCallback(int wd, unsigned mask, const char *name) noexcept;
+
+	static void InotifyCallback(int wd, unsigned mask,
+				    const char *name, void *ctx) noexcept {
+		auto &iu = *(InotifyUpdate *)ctx;
+		iu.InotifyCallback(wd, mask, name);
+	}
+
+	void AddToMap(WatchDirectory &directory) noexcept;
+	void RemoveFromMap(WatchDirectory &directory) noexcept;
+	void Disable(WatchDirectory &directory) noexcept;
+	void Delete(WatchDirectory &directory) noexcept;
+
+	void RecursiveWatchSubdirectories(WatchDirectory &parent,
+					  Path path_fs,
+					  unsigned depth) noexcept;
+};
+
+/**
+ * Throws on error.
+ */
+std::unique_ptr<InotifyUpdate>
 mpd_inotify_init(EventLoop &loop, Storage &storage, UpdateService &update,
 		 unsigned max_depth);
 
-void
-mpd_inotify_finish() noexcept;
-
 #endif