From c598686bd90aa49285e3e6bb7ad222231e1d3995 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 27 Oct 2016 08:40:40 +0200 Subject: [PATCH] storage: migrate from class Error to C++ exceptions --- src/Main.cxx | 14 ++--- src/SongUpdate.cxx | 4 +- src/command/StorageCommands.cxx | 49 ++++------------ src/db/update/UpdateIO.cxx | 31 ++++------ src/db/update/Walk.cxx | 7 +-- src/storage/CompositeStorage.cxx | 65 ++++++++++---------- src/storage/CompositeStorage.hxx | 8 +-- src/storage/Configured.cxx | 33 ++++++----- src/storage/Configured.hxx | 8 +-- src/storage/MemoryDirectoryReader.cxx | 9 +-- src/storage/MemoryDirectoryReader.hxx | 3 +- src/storage/Registry.cxx | 9 +-- src/storage/Registry.hxx | 3 +- src/storage/StorageInterface.hxx | 23 +++++--- src/storage/StoragePlugin.hxx | 7 ++- src/storage/plugins/LocalStorage.cxx | 66 ++++++++------------- src/storage/plugins/NfsStorage.cxx | 43 ++++++-------- src/storage/plugins/SmbclientStorage.cxx | 75 +++++++++++------------- test/run_storage.cxx | 32 ++++------ 19 files changed, 208 insertions(+), 281 deletions(-) diff --git a/src/Main.cxx b/src/Main.cxx index 5f3d56e4c..d3e141d2f 100644 --- a/src/Main.cxx +++ b/src/Main.cxx @@ -161,19 +161,16 @@ glue_mapper_init(Error &error) #ifdef ENABLE_DATABASE -static bool -InitStorage(Error &error) +static void +InitStorage() { - Storage *storage = CreateConfiguredStorage(io_thread_get(), error); + Storage *storage = CreateConfiguredStorage(io_thread_get()); if (storage == nullptr) - return !error.IsDefined(); - - assert(!error.IsDefined()); + return; CompositeStorage *composite = new CompositeStorage(); instance->storage = composite; composite->Mount("", storage); - return true; } /** @@ -196,8 +193,7 @@ glue_db_init_and_load(void) } if (instance->database->GetPlugin().flags & DatabasePlugin::FLAG_REQUIRE_STORAGE) { - if (!InitStorage(error)) - FatalError(error); + InitStorage(); if (instance->storage == nullptr) { delete instance->database; diff --git a/src/SongUpdate.cxx b/src/SongUpdate.cxx index 78741dc3e..bbfc7ad09 100644 --- a/src/SongUpdate.cxx +++ b/src/SongUpdate.cxx @@ -68,9 +68,7 @@ Song::UpdateFile(Storage &storage) StorageFileInfo info; try { - if (!storage.GetInfo(relative_uri.c_str(), true, info, - IgnoreError())) - return false; + info = storage.GetInfo(relative_uri.c_str(), true); } catch (const std::runtime_error &) { return false; } diff --git a/src/command/StorageCommands.cxx b/src/command/StorageCommands.cxx index bbb22d132..48e1033fa 100644 --- a/src/command/StorageCommands.cxx +++ b/src/command/StorageCommands.cxx @@ -58,9 +58,8 @@ skip_path(const char *name_utf8) #pragma GCC diagnostic ignored "-Wformat-extra-args" #endif -static bool -handle_listfiles_storage(Response &r, StorageDirectoryReader &reader, - Error &error) +static void +handle_listfiles_storage(Response &r, StorageDirectoryReader &reader) { const char *name_utf8; while ((name_utf8 = reader.Read()) != nullptr) { @@ -68,8 +67,11 @@ handle_listfiles_storage(Response &r, StorageDirectoryReader &reader, continue; StorageFileInfo info; - if (!reader.GetInfo(false, info, error)) + try { + info = reader.GetInfo(false); + } catch (const std::runtime_error &) { continue; + } switch (info.type) { case StorageFileInfo::Type::OTHER: @@ -91,53 +93,30 @@ handle_listfiles_storage(Response &r, StorageDirectoryReader &reader, if (info.mtime != 0) time_print(r, "Last-Modified", info.mtime); } - - return true; } #if defined(WIN32) && GCC_CHECK_VERSION(4,6) #pragma GCC diagnostic pop #endif -static bool -handle_listfiles_storage(Response &r, Storage &storage, const char *uri, - Error &error) -{ - std::unique_ptr reader(storage.OpenDirectory(uri, error)); - if (reader == nullptr) - return false; - - return handle_listfiles_storage(r, *reader, error); -} - CommandResult handle_listfiles_storage(Response &r, Storage &storage, const char *uri) { - Error error; - if (!handle_listfiles_storage(r, storage, uri, error)) - return print_error(r, error); - + std::unique_ptr reader(storage.OpenDirectory(uri)); + handle_listfiles_storage(r, *reader); return CommandResult::OK; } CommandResult handle_listfiles_storage(Response &r, const char *uri) { - Error error; - std::unique_ptr storage(CreateStorageURI(io_thread_get(), uri, - error)); + std::unique_ptr storage(CreateStorageURI(io_thread_get(), uri)); if (storage == nullptr) { - if (error.IsDefined()) - return print_error(r, error); - r.Error(ACK_ERROR_ARG, "Unrecognized storage URI"); return CommandResult::ERROR; } - if (!handle_listfiles_storage(r, *storage, "", error)) - return print_error(r, error); - - return CommandResult::OK; + return handle_listfiles_storage(r, *storage, ""); } static void @@ -217,13 +196,8 @@ handle_mount(Client &client, Request args, Response &r) return CommandResult::ERROR; } - Error error; - Storage *storage = CreateStorageURI(io_thread_get(), remote_uri, - error); + Storage *storage = CreateStorageURI(io_thread_get(), remote_uri); if (storage == nullptr) { - if (error.IsDefined()) - return print_error(r, error); - r.Error(ACK_ERROR_ARG, "Unrecognized storage URI"); return CommandResult::ERROR; } @@ -237,6 +211,7 @@ handle_mount(Client &client, Request args, Response &r) SimpleDatabase &db = *(SimpleDatabase *)_db; try { + Error error; if (!db.Mount(local_uri, remote_uri, error)) { composite.Unmount(local_uri); return print_error(r, error); diff --git a/src/db/update/UpdateIO.cxx b/src/db/update/UpdateIO.cxx index da6e4a096..9558c93d9 100644 --- a/src/db/update/UpdateIO.cxx +++ b/src/db/update/UpdateIO.cxx @@ -35,11 +35,8 @@ bool GetInfo(Storage &storage, const char *uri_utf8, StorageFileInfo &info) try { - Error error; - bool success = storage.GetInfo(uri_utf8, true, info, error); - if (!success) - LogError(error); - return success; + info = storage.GetInfo(uri_utf8, true); + return true; } catch (const std::runtime_error &e) { LogError(e); return false; @@ -48,11 +45,8 @@ try { bool GetInfo(StorageDirectoryReader &reader, StorageFileInfo &info) try { - Error error; - bool success = reader.GetInfo(true, info, error); - if (!success) - LogError(error); - return success; + info = reader.GetInfo(true); + return true; } catch (const std::runtime_error &e) { LogError(e); return false; @@ -64,8 +58,7 @@ DirectoryExists(Storage &storage, const Directory &directory) StorageFileInfo info; try { - if (!storage.GetInfo(directory.GetPath(), true, info, IgnoreError())) - return false; + info = storage.GetInfo(directory.GetPath(), true); } catch (const std::runtime_error &) { return false; } @@ -76,24 +69,22 @@ DirectoryExists(Storage &storage, const Directory &directory) : info.IsDirectory(); } -static bool +static StorageFileInfo GetDirectoryChildInfo(Storage &storage, const Directory &directory, - const char *name_utf8, StorageFileInfo &info, Error &error) + const char *name_utf8) { const auto uri_utf8 = PathTraitsUTF8::Build(directory.GetPath(), name_utf8); - return storage.GetInfo(uri_utf8.c_str(), true, info, error); + return storage.GetInfo(uri_utf8.c_str(), true); } bool directory_child_is_regular(Storage &storage, const Directory &directory, const char *name_utf8) try { - StorageFileInfo info; - return GetDirectoryChildInfo(storage, directory, name_utf8, info, - IgnoreError()) && - info.IsRegular(); - } catch (const std::runtime_error &) { + return GetDirectoryChildInfo(storage, directory, name_utf8) + .IsRegular(); +} catch (const std::runtime_error &) { return false; } diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index f9a697fdc..0a7ff1367 100644 --- a/src/db/update/Walk.cxx +++ b/src/db/update/Walk.cxx @@ -338,12 +338,7 @@ UpdateWalk::UpdateDirectory(Directory &directory, std::unique_ptr reader; try { - Error error; - reader.reset(storage.OpenDirectory(directory.GetPath(), error)); - if (!reader) { - LogError(error); - return false; - } + reader.reset(storage.OpenDirectory(directory.GetPath())); } catch (const std::runtime_error &e) { LogError(e); return false; diff --git a/src/storage/CompositeStorage.cxx b/src/storage/CompositeStorage.cxx index bc0b9d56e..e46d10f81 100644 --- a/src/storage/CompositeStorage.cxx +++ b/src/storage/CompositeStorage.cxx @@ -21,16 +21,13 @@ #include "CompositeStorage.hxx" #include "FileInfo.hxx" #include "fs/AllocatedPath.hxx" -#include "util/Error.hxx" -#include "util/Domain.hxx" #include "util/StringCompare.hxx" #include +#include #include -static constexpr Domain composite_domain("composite"); - /** * Combines the directory entries of another #StorageDirectoryReader * instance and the virtual directory entries. @@ -57,7 +54,7 @@ public: /* virtual methods from class StorageDirectoryReader */ const char *Read() override; - bool GetInfo(bool follow, StorageFileInfo &info, Error &error) override; + StorageFileInfo GetInfo(bool follow) override; }; const char * @@ -81,20 +78,20 @@ CompositeDirectoryReader::Read() return current->c_str(); } -bool -CompositeDirectoryReader::GetInfo(bool follow, StorageFileInfo &info, - Error &error) +StorageFileInfo +CompositeDirectoryReader::GetInfo(bool follow) { if (other != nullptr) - return other->GetInfo(follow, info, error); + return other->GetInfo(follow); assert(current != names.end()); + StorageFileInfo info; info.type = StorageFileInfo::Type::DIRECTORY; info.mtime = 0; info.device = 0; info.inode = 0; - return true; + return info; } static std::string @@ -266,35 +263,40 @@ CompositeStorage::FindStorage(const char *uri) const return result; } -bool -CompositeStorage::GetInfo(const char *uri, bool follow, StorageFileInfo &info, - Error &error) +StorageFileInfo +CompositeStorage::GetInfo(const char *uri, bool follow) { const ScopeLock protect(mutex); + std::exception_ptr error; + auto f = FindStorage(uri); - if (f.directory->storage != nullptr && - f.directory->storage->GetInfo(f.uri, follow, info, error)) - return true; + if (f.directory->storage != nullptr) { + try { + return f.directory->storage->GetInfo(f.uri, follow); + } catch (...) { + error = std::current_exception(); + } + } const Directory *directory = f.directory->Find(f.uri); if (directory != nullptr) { - error.Clear(); + StorageFileInfo info; info.type = StorageFileInfo::Type::DIRECTORY; info.mtime = 0; info.device = 0; info.inode = 0; - return true; + return info; } - if (!error.IsDefined()) - error.Set(composite_domain, "No such directory"); - return false; + if (error) + std::rethrow_exception(error); + else + throw std::runtime_error("No such file or directory"); } StorageDirectoryReader * -CompositeStorage::OpenDirectory(const char *uri, - Error &error) +CompositeStorage::OpenDirectory(const char *uri) { const ScopeLock protect(mutex); @@ -303,16 +305,19 @@ CompositeStorage::OpenDirectory(const char *uri, if (directory == nullptr || directory->children.empty()) { /* no virtual directories here */ - if (f.directory->storage == nullptr) { - error.Set(composite_domain, "No such directory"); - return nullptr; - } + if (f.directory->storage == nullptr) + throw std::runtime_error("No such directory"); - return f.directory->storage->OpenDirectory(f.uri, error); + return f.directory->storage->OpenDirectory(f.uri); + } + + StorageDirectoryReader *other = nullptr; + + try { + other = f.directory->storage->OpenDirectory(f.uri); + } catch (const std::runtime_error &) { } - StorageDirectoryReader *other = - f.directory->storage->OpenDirectory(f.uri, IgnoreError()); return new CompositeDirectoryReader(other, directory->children); } diff --git a/src/storage/CompositeStorage.hxx b/src/storage/CompositeStorage.hxx index 05d6b3130..65d96b354 100644 --- a/src/storage/CompositeStorage.hxx +++ b/src/storage/CompositeStorage.hxx @@ -28,8 +28,6 @@ #include #include -class Error; - /** * A #Storage implementation that combines multiple other #Storage * instances in one virtual tree. It is used to "mount" new #Storage @@ -121,11 +119,9 @@ public: bool Unmount(const char *uri); /* virtual methods from class Storage */ - bool GetInfo(const char *uri, bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(const char *uri, bool follow) override; - StorageDirectoryReader *OpenDirectory(const char *uri, - Error &error) override; + StorageDirectoryReader *OpenDirectory(const char *uri) override; std::string MapUTF8(const char *uri) const override; diff --git a/src/storage/Configured.cxx b/src/storage/Configured.cxx index 076c63c13..82e7cd603 100644 --- a/src/storage/Configured.cxx +++ b/src/storage/Configured.cxx @@ -27,34 +27,39 @@ #include "fs/CheckFile.hxx" #include "util/UriUtil.hxx" #include "util/Error.hxx" +#include "util/RuntimeError.hxx" #include static Storage * -CreateConfiguredStorageUri(EventLoop &event_loop, const char *uri, - Error &error) +CreateConfiguredStorageUri(EventLoop &event_loop, const char *uri) { - Storage *storage = CreateStorageURI(event_loop, uri, error); - if (storage == nullptr && !error.IsDefined()) - error.Format(config_domain, - "Unrecognized storage URI: %s", uri); + Storage *storage = CreateStorageURI(event_loop, uri); + if (storage == nullptr) + throw FormatRuntimeError("Unrecognized storage URI: %s", uri); + return storage; } static AllocatedPath -GetConfiguredMusicDirectory(Error &error) +GetConfiguredMusicDirectory() { + Error error; AllocatedPath path = config_get_path(ConfigOption::MUSIC_DIR, error); - if (path.IsNull() && !error.IsDefined()) + if (path.IsNull()) { + if (error.IsDefined()) + throw std::runtime_error(error.GetMessage()); + path = GetUserMusicDir(); + } return path; } static Storage * -CreateConfiguredStorageLocal(Error &error) +CreateConfiguredStorageLocal() { - AllocatedPath path = GetConfiguredMusicDirectory(error); + AllocatedPath path = GetConfiguredMusicDirectory(); if (path.IsNull()) return nullptr; @@ -64,15 +69,13 @@ CreateConfiguredStorageLocal(Error &error) } Storage * -CreateConfiguredStorage(EventLoop &event_loop, Error &error) +CreateConfiguredStorage(EventLoop &event_loop) { - assert(!error.IsDefined()); - auto uri = config_get_string(ConfigOption::MUSIC_DIR); if (uri != nullptr && uri_has_scheme(uri)) - return CreateConfiguredStorageUri(event_loop, uri, error); + return CreateConfiguredStorageUri(event_loop, uri); - return CreateConfiguredStorageLocal(error); + return CreateConfiguredStorageLocal(); } bool diff --git a/src/storage/Configured.hxx b/src/storage/Configured.hxx index bd5eebf96..f74cd6bff 100644 --- a/src/storage/Configured.hxx +++ b/src/storage/Configured.hxx @@ -23,17 +23,17 @@ #include "check.h" #include "Compiler.h" -class Error; class Storage; class EventLoop; /** * Read storage configuration settings and create a #Storage instance - * from it. Returns nullptr on error or if no storage is configured - * (no #Error set in that case). + * from it. Returns nullptr if no storage is configured. + * + * Throws #std::runtime_error on error. */ Storage * -CreateConfiguredStorage(EventLoop &event_loop, Error &error); +CreateConfiguredStorage(EventLoop &event_loop); /** * Returns true if there is configuration for a #Storage instance. diff --git a/src/storage/MemoryDirectoryReader.cxx b/src/storage/MemoryDirectoryReader.cxx index 262cba677..d69b389ae 100644 --- a/src/storage/MemoryDirectoryReader.cxx +++ b/src/storage/MemoryDirectoryReader.cxx @@ -36,14 +36,11 @@ MemoryStorageDirectoryReader::Read() return entries.front().name.c_str(); } -bool -MemoryStorageDirectoryReader::GetInfo(gcc_unused bool follow, - StorageFileInfo &info, - gcc_unused Error &error) +StorageFileInfo +MemoryStorageDirectoryReader::GetInfo(gcc_unused bool follow) { assert(!first); assert(!entries.empty()); - info = entries.front().info; - return true; + return entries.front().info; } diff --git a/src/storage/MemoryDirectoryReader.hxx b/src/storage/MemoryDirectoryReader.hxx index 44c670b49..1450e4788 100644 --- a/src/storage/MemoryDirectoryReader.hxx +++ b/src/storage/MemoryDirectoryReader.hxx @@ -61,8 +61,7 @@ public: /* virtual methods from class StorageDirectoryReader */ const char *Read() override; - bool GetInfo(bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(bool follow) override; }; #endif diff --git a/src/storage/Registry.cxx b/src/storage/Registry.cxx index c82e71154..3e6431549 100644 --- a/src/storage/Registry.cxx +++ b/src/storage/Registry.cxx @@ -23,7 +23,6 @@ #include "plugins/LocalStorage.hxx" #include "plugins/SmbclientStorage.hxx" #include "plugins/NfsStorage.hxx" -#include "util/Error.hxx" #include #include @@ -52,18 +51,16 @@ GetStoragePluginByName(const char *name) } Storage * -CreateStorageURI(EventLoop &event_loop, const char *uri, Error &error) +CreateStorageURI(EventLoop &event_loop, const char *uri) { - assert(!error.IsDefined()); - for (auto i = storage_plugins; *i != nullptr; ++i) { const StoragePlugin &plugin = **i; if (plugin.create_uri == nullptr) continue; - Storage *storage = plugin.create_uri(event_loop, uri, error); - if (storage != nullptr || error.IsDefined()) + Storage *storage = plugin.create_uri(event_loop, uri); + if (storage != nullptr) return storage; } diff --git a/src/storage/Registry.hxx b/src/storage/Registry.hxx index 28b0ff6e9..c9f226b51 100644 --- a/src/storage/Registry.hxx +++ b/src/storage/Registry.hxx @@ -25,7 +25,6 @@ struct StoragePlugin; class Storage; -class Error; class EventLoop; /** @@ -40,6 +39,6 @@ GetStoragePluginByName(const char *name); gcc_nonnull_all gcc_malloc Storage * -CreateStorageURI(EventLoop &event_loop, const char *uri, Error &error); +CreateStorageURI(EventLoop &event_loop, const char *uri); #endif diff --git a/src/storage/StorageInterface.hxx b/src/storage/StorageInterface.hxx index f007748d9..c7597be63 100644 --- a/src/storage/StorageInterface.hxx +++ b/src/storage/StorageInterface.hxx @@ -27,7 +27,6 @@ struct StorageFileInfo; class AllocatedPath; -class Error; class StorageDirectoryReader { public: @@ -36,8 +35,12 @@ public: virtual ~StorageDirectoryReader() {} virtual const char *Read() = 0; - virtual bool GetInfo(bool follow, StorageFileInfo &info, - Error &error) = 0; + + /** + * Throws #std::runtime_error on error. + */ + gcc_pure + virtual StorageFileInfo GetInfo(bool follow) = 0; }; class Storage { @@ -46,12 +49,16 @@ public: Storage(const Storage &) = delete; virtual ~Storage() {} - virtual bool GetInfo(const char *uri_utf8, bool follow, - StorageFileInfo &info, - Error &error) = 0; + /** + * Throws #std::runtime_error on error. + */ + gcc_pure + virtual StorageFileInfo GetInfo(const char *uri_utf8, bool follow) = 0; - virtual StorageDirectoryReader *OpenDirectory(const char *uri_utf8, - Error &error) = 0; + /** + * Throws #std::runtime_error on error. + */ + virtual StorageDirectoryReader *OpenDirectory(const char *uri_utf8) = 0; /** * Map the given relative URI to an absolute URI. diff --git a/src/storage/StoragePlugin.hxx b/src/storage/StoragePlugin.hxx index 1fdff030e..f5b5bfab4 100644 --- a/src/storage/StoragePlugin.hxx +++ b/src/storage/StoragePlugin.hxx @@ -22,15 +22,16 @@ #include "check.h" -class Error; class Storage; class EventLoop; struct StoragePlugin { const char *name; - Storage *(*create_uri)(EventLoop &event_loop, const char *uri, - Error &error); + /** + * Throws #std::runtime_error on error. + */ + Storage *(*create_uri)(EventLoop &event_loop, const char *uri); }; #endif diff --git a/src/storage/plugins/LocalStorage.cxx b/src/storage/plugins/LocalStorage.cxx index 191235975..35c783ed0 100644 --- a/src/storage/plugins/LocalStorage.cxx +++ b/src/storage/plugins/LocalStorage.cxx @@ -25,7 +25,6 @@ #include "fs/FileInfo.hxx" #include "fs/AllocatedPath.hxx" #include "fs/DirectoryReader.hxx" -#include "util/Error.hxx" #include "util/StringCompare.hxx" #include @@ -43,8 +42,7 @@ public: /* virtual methods from class StorageDirectoryReader */ const char *Read() override; - bool GetInfo(bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(bool follow) override; }; class LocalStorage final : public Storage { @@ -59,11 +57,9 @@ public: } /* virtual methods from class Storage */ - bool GetInfo(const char *uri_utf8, bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(const char *uri_utf8, bool follow) override; - StorageDirectoryReader *OpenDirectory(const char *uri_utf8, - Error &error) override; + StorageDirectoryReader *OpenDirectory(const char *uri_utf8) override; std::string MapUTF8(const char *uri_utf8) const override; @@ -72,15 +68,15 @@ public: const char *MapToRelativeUTF8(const char *uri_utf8) const override; private: - AllocatedPath MapFS(const char *uri_utf8, Error &error) const; + AllocatedPath MapFSOrThrow(const char *uri_utf8) const; }; -static bool -Stat(Path path, bool follow, StorageFileInfo &info, Error &error) +gcc_pure +static StorageFileInfo +Stat(Path path, bool follow) { - FileInfo src; - if (!GetFileInfo(path, src, follow, error)) - return false; + const FileInfo src(path, follow); + StorageFileInfo info; if (src.IsRegular()) info.type = StorageFileInfo::Type::REGULAR; @@ -97,7 +93,7 @@ Stat(Path path, bool follow, StorageFileInfo &info, Error &error) info.device = src.GetDevice(); info.inode = src.GetInode(); #endif - return true; + return info; } std::string @@ -112,24 +108,25 @@ LocalStorage::MapUTF8(const char *uri_utf8) const } AllocatedPath -LocalStorage::MapFS(const char *uri_utf8, Error &error) const +LocalStorage::MapFSOrThrow(const char *uri_utf8) const { assert(uri_utf8 != nullptr); if (StringIsEmpty(uri_utf8)) return base_fs; - AllocatedPath path_fs = AllocatedPath::FromUTF8(uri_utf8, error); - if (!path_fs.IsNull()) - path_fs = AllocatedPath::Build(base_fs, path_fs); - - return path_fs; + return AllocatedPath::Build(base_fs, + AllocatedPath::FromUTF8Throw(uri_utf8)); } AllocatedPath LocalStorage::MapFS(const char *uri_utf8) const { - return MapFS(uri_utf8, IgnoreError()); + try { + return MapFSOrThrow(uri_utf8); + } catch (const std::runtime_error &) { + return AllocatedPath::Null(); + } } const char * @@ -138,25 +135,16 @@ LocalStorage::MapToRelativeUTF8(const char *uri_utf8) const return PathTraitsUTF8::Relative(base_utf8.c_str(), uri_utf8); } -bool -LocalStorage::GetInfo(const char *uri_utf8, bool follow, StorageFileInfo &info, - Error &error) +StorageFileInfo +LocalStorage::GetInfo(const char *uri_utf8, bool follow) { - AllocatedPath path_fs = MapFS(uri_utf8, error); - if (path_fs.IsNull()) - return false; - - return Stat(path_fs, follow, info, error); + return Stat(MapFSOrThrow(uri_utf8), follow); } StorageDirectoryReader * -LocalStorage::OpenDirectory(const char *uri_utf8, Error &error) +LocalStorage::OpenDirectory(const char *uri_utf8) { - AllocatedPath path_fs = MapFS(uri_utf8, error); - if (path_fs.IsNull()) - return nullptr; - - return new LocalDirectoryReader(std::move(path_fs)); + return new LocalDirectoryReader(MapFSOrThrow(uri_utf8)); } gcc_pure @@ -186,12 +174,10 @@ LocalDirectoryReader::Read() return nullptr; } -bool -LocalDirectoryReader::GetInfo(bool follow, StorageFileInfo &info, Error &error) +StorageFileInfo +LocalDirectoryReader::GetInfo(bool follow) { - const AllocatedPath path_fs = - AllocatedPath::Build(base_fs, reader.GetEntry()); - return Stat(path_fs, follow, info, error); + return Stat(AllocatedPath::Build(base_fs, reader.GetEntry()), follow); } Storage * diff --git a/src/storage/plugins/NfsStorage.cxx b/src/storage/plugins/NfsStorage.cxx index 4e3df27ef..ba2e54556 100644 --- a/src/storage/plugins/NfsStorage.cxx +++ b/src/storage/plugins/NfsStorage.cxx @@ -35,7 +35,6 @@ #include "event/Call.hxx" #include "event/DeferredMonitor.hxx" #include "event/TimeoutMonitor.hxx" -#include "util/Error.hxx" #include "util/StringCompare.hxx" extern "C" { @@ -84,11 +83,9 @@ public: } /* virtual methods from class Storage */ - bool GetInfo(const char *uri_utf8, bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(const char *uri_utf8, bool follow) override; - StorageDirectoryReader *OpenDirectory(const char *uri_utf8, - Error &error) override; + StorageDirectoryReader *OpenDirectory(const char *uri_utf8) override; std::string MapUTF8(const char *uri_utf8) const override; @@ -214,7 +211,7 @@ private: }; static std::string -UriToNfsPath(const char *_uri_utf8, Error &error) +UriToNfsPath(const char *_uri_utf8) { assert(_uri_utf8 != nullptr); @@ -222,7 +219,7 @@ UriToNfsPath(const char *_uri_utf8, Error &error) std::string uri_utf8("/"); uri_utf8.append(_uri_utf8); - return AllocatedPath::FromUTF8(uri_utf8.c_str(), error).Steal(); + return AllocatedPath::FromUTF8Throw(uri_utf8.c_str()).Steal(); } std::string @@ -260,12 +257,15 @@ Copy(StorageFileInfo &info, const struct stat &st) class NfsGetInfoOperation final : public BlockingNfsOperation { const char *const path; - StorageFileInfo &info; + StorageFileInfo info; public: - NfsGetInfoOperation(NfsConnection &_connection, const char *_path, - StorageFileInfo &_info) - :BlockingNfsOperation(_connection), path(_path), info(_info) {} + NfsGetInfoOperation(NfsConnection &_connection, const char *_path) + :BlockingNfsOperation(_connection), path(_path) {} + + const StorageFileInfo &GetInfo() const { + return info; + } protected: void Start() override { @@ -277,19 +277,16 @@ protected: } }; -bool -NfsStorage::GetInfo(const char *uri_utf8, gcc_unused bool follow, - StorageFileInfo &info, Error &error) +StorageFileInfo +NfsStorage::GetInfo(const char *uri_utf8, gcc_unused bool follow) { - const std::string path = UriToNfsPath(uri_utf8, error); - if (path.empty()) - return false; + const std::string path = UriToNfsPath(uri_utf8); WaitConnected(); - NfsGetInfoOperation operation(*connection, path.c_str(), info); + NfsGetInfoOperation operation(*connection, path.c_str()); operation.Run(); - return true; + return operation.GetInfo(); } gcc_pure @@ -377,11 +374,9 @@ NfsListDirectoryOperation::CollectEntries(struct nfsdir *dir) } StorageDirectoryReader * -NfsStorage::OpenDirectory(const char *uri_utf8, Error &error) +NfsStorage::OpenDirectory(const char *uri_utf8) { - const std::string path = UriToNfsPath(uri_utf8, error); - if (path.empty()) - return nullptr; + const std::string path = UriToNfsPath(uri_utf8); WaitConnected(); @@ -392,7 +387,7 @@ NfsStorage::OpenDirectory(const char *uri_utf8, Error &error) } static Storage * -CreateNfsStorageURI(EventLoop &event_loop, const char *base, Error &) +CreateNfsStorageURI(EventLoop &event_loop, const char *base) { if (memcmp(base, "nfs://", 6) != 0) return nullptr; diff --git a/src/storage/plugins/SmbclientStorage.cxx b/src/storage/plugins/SmbclientStorage.cxx index 86b727315..0fe5b128b 100644 --- a/src/storage/plugins/SmbclientStorage.cxx +++ b/src/storage/plugins/SmbclientStorage.cxx @@ -26,8 +26,9 @@ #include "lib/smbclient/Mutex.hxx" #include "fs/Traits.hxx" #include "thread/Mutex.hxx" -#include "util/Error.hxx" +#include "system/Error.hxx" #include "util/StringCompare.hxx" +#include "util/ScopeExit.hxx" #include @@ -45,8 +46,7 @@ public: /* virtual methods from class StorageDirectoryReader */ const char *Read() override; - bool GetInfo(bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(bool follow) override; }; class SmbclientStorage final : public Storage { @@ -65,11 +65,9 @@ public: } /* virtual methods from class Storage */ - bool GetInfo(const char *uri_utf8, bool follow, StorageFileInfo &info, - Error &error) override; + StorageFileInfo GetInfo(const char *uri_utf8, bool follow) override; - StorageDirectoryReader *OpenDirectory(const char *uri_utf8, - Error &error) override; + StorageDirectoryReader *OpenDirectory(const char *uri_utf8) override; std::string MapUTF8(const char *uri_utf8) const override; @@ -93,18 +91,18 @@ SmbclientStorage::MapToRelativeUTF8(const char *uri_utf8) const return PathTraitsUTF8::Relative(base.c_str(), uri_utf8); } -static bool -GetInfo(const char *path, StorageFileInfo &info, Error &error) +static StorageFileInfo +GetInfo(const char *path) { struct stat st; - smbclient_mutex.lock(); - bool success = smbc_stat(path, &st) == 0; - smbclient_mutex.unlock(); - if (!success) { - error.SetErrno(); - return false; + + { + const ScopeLock protect(smbclient_mutex); + if (smbc_stat(path, &st) != 0) + throw MakeErrno("Failed to access file"); } + StorageFileInfo info; if (S_ISREG(st.st_mode)) info.type = StorageFileInfo::Type::REGULAR; else if (S_ISDIR(st.st_mode)) @@ -116,27 +114,28 @@ GetInfo(const char *path, StorageFileInfo &info, Error &error) info.mtime = st.st_mtime; info.device = st.st_dev; info.inode = st.st_ino; - return true; + return info; } -bool -SmbclientStorage::GetInfo(const char *uri_utf8, gcc_unused bool follow, - StorageFileInfo &info, Error &error) +StorageFileInfo +SmbclientStorage::GetInfo(const char *uri_utf8, gcc_unused bool follow) { const std::string mapped = MapUTF8(uri_utf8); - return ::GetInfo(mapped.c_str(), info, error); + return ::GetInfo(mapped.c_str()); } StorageDirectoryReader * -SmbclientStorage::OpenDirectory(const char *uri_utf8, Error &error) +SmbclientStorage::OpenDirectory(const char *uri_utf8) { std::string mapped = MapUTF8(uri_utf8); - smbclient_mutex.lock(); - int handle = smbc_opendir(mapped.c_str()); - smbclient_mutex.unlock(); - if (handle < 0) { - error.SetErrno(); - return nullptr; + + int handle; + + { + const ScopeLock protect(smbclient_mutex); + handle = smbc_opendir(mapped.c_str()); + if (handle < 0) + throw MakeErrno("Failed to open directory"); } return new SmbclientDirectoryReader(std::move(mapped.c_str()), handle); @@ -173,18 +172,15 @@ SmbclientDirectoryReader::Read() return nullptr; } -bool -SmbclientDirectoryReader::GetInfo(gcc_unused bool follow, - StorageFileInfo &info, - Error &error) +StorageFileInfo +SmbclientDirectoryReader::GetInfo(gcc_unused bool follow) { const std::string path = PathTraitsUTF8::Build(base.c_str(), name); - return ::GetInfo(path.c_str(), info, error); + return ::GetInfo(path.c_str()); } static Storage * -CreateSmbclientStorageURI(gcc_unused EventLoop &event_loop, const char *base, - Error &error) +CreateSmbclientStorageURI(gcc_unused EventLoop &event_loop, const char *base) { if (memcmp(base, "smb://", 6) != 0) return nullptr; @@ -193,16 +189,13 @@ CreateSmbclientStorageURI(gcc_unused EventLoop &event_loop, const char *base, const ScopeLock protect(smbclient_mutex); SMBCCTX *ctx = smbc_new_context(); - if (ctx == nullptr) { - error.SetErrno("smbc_new_context() failed"); - return nullptr; - } + if (ctx == nullptr) + throw MakeErrno("smbc_new_context() failed"); SMBCCTX *ctx2 = smbc_init_context(ctx); if (ctx2 == nullptr) { - error.SetErrno("smbc_init_context() failed"); - smbc_free_context(ctx, 1); - return nullptr; + AtScopeExit(ctx) { smbc_free_context(ctx, 1); }; + throw MakeErrno("smbc_new_context() failed"); } return new SmbclientStorage(base, ctx2); diff --git a/test/run_storage.cxx b/test/run_storage.cxx index 592c691af..81fb04cd3 100644 --- a/test/run_storage.cxx +++ b/test/run_storage.cxx @@ -18,6 +18,7 @@ */ #include "config.h" +#include "Log.hxx" #include "ScopeIOThread.hxx" #include "storage/Registry.hxx" #include "storage/StorageInterface.hxx" @@ -25,6 +26,7 @@ #include "util/Error.hxx" #include +#include #include #include @@ -35,12 +37,9 @@ static Storage * MakeStorage(const char *uri) { - Error error; - Storage *storage = CreateStorageURI(io_thread_get(), uri, error); - if (storage == nullptr) { - fprintf(stderr, "%s\n", error.GetMessage()); - exit(EXIT_FAILURE); - } + Storage *storage = CreateStorageURI(io_thread_get(), uri); + if (storage == nullptr) + throw std::runtime_error("Unrecognized storage URI"); return storage; } @@ -48,21 +47,11 @@ MakeStorage(const char *uri) static int Ls(Storage &storage, const char *path) { - Error error; - auto dir = storage.OpenDirectory(path, error); - if (dir == nullptr) { - fprintf(stderr, "%s\n", error.GetMessage()); - return EXIT_FAILURE; - } + auto dir = storage.OpenDirectory(path); const char *name; while ((name = dir->Read()) != nullptr) { - StorageFileInfo info; - if (!dir->GetInfo(false, info, error)) { - printf("Error on %s: %s\n", name, error.GetMessage()); - error.Clear(); - continue; - } + const auto info = dir->GetInfo(false); const char *type = "unk"; switch (info.type) { @@ -93,7 +82,7 @@ Ls(Storage &storage, const char *path) int main(int argc, char **argv) -{ +try { if (argc < 3) { fprintf(stderr, "Usage: run_storage COMMAND URI ...\n"); return EXIT_FAILURE; @@ -119,4 +108,9 @@ main(int argc, char **argv) fprintf(stderr, "Unknown command\n"); return EXIT_FAILURE; } + + return EXIT_SUCCESS; +} catch (const std::exception &e) { + LogError(e); + return EXIT_FAILURE; }