Browse Source

LibCore+Everywhere: Return ErrorOr from ConfigFile::sync()

Currently this method always succeeds, but that won't be true once we
switch to the Core::Stream API. :^)

Some of these places would ideally show an error message to the user,
since failure to save a file is significant, but let's not get
distracted right now.
Sam Atkins 3 years ago
parent
commit
cd0ffe5460

+ 4 - 1
Userland/Applications/ThemeEditor/main.cpp

@@ -374,7 +374,10 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
             theme->write_entry("Paths", to_string(role), preview_widget.preview_palette().path(role));
             theme->write_entry("Paths", to_string(role), preview_widget.preview_palette().path(role));
         }
         }
 
 
-        theme->sync();
+        if (auto sync_result = theme->sync(); sync_result.is_error()) {
+            // FIXME: Expose this to the user, since failing to save is important to know about!
+            dbgln("Failed to save theme file: {}", sync_result.error());
+        }
     };
     };
 
 
     TRY(file_menu->try_add_action(GUI::CommonActions::make_open_action([&](auto&) {
     TRY(file_menu->try_add_action(GUI::CommonActions::make_open_action([&](auto&) {

+ 4 - 4
Userland/Libraries/LibCore/ConfigFile.cpp

@@ -61,7 +61,7 @@ ConfigFile::ConfigFile(String const&, NonnullRefPtr<File> open_file)
 
 
 ConfigFile::~ConfigFile()
 ConfigFile::~ConfigFile()
 {
 {
-    sync();
+    MUST(sync());
 }
 }
 
 
 void ConfigFile::reparse()
 void ConfigFile::reparse()
@@ -168,10 +168,10 @@ void ConfigFile::write_color_entry(String const& group, String const& key, Color
     write_entry(group, key, String::formatted("{},{},{},{}", value.red(), value.green(), value.blue(), value.alpha()));
     write_entry(group, key, String::formatted("{},{},{},{}", value.red(), value.green(), value.blue(), value.alpha()));
 }
 }
 
 
-bool ConfigFile::sync()
+ErrorOr<void> ConfigFile::sync()
 {
 {
     if (!m_dirty)
     if (!m_dirty)
-        return true;
+        return {};
 
 
     m_file->truncate(0);
     m_file->truncate(0);
     m_file->seek(0);
     m_file->seek(0);
@@ -184,7 +184,7 @@ bool ConfigFile::sync()
     }
     }
 
 
     m_dirty = false;
     m_dirty = false;
-    return true;
+    return {};
 }
 }
 
 
 void ConfigFile::dump() const
 void ConfigFile::dump() const

+ 1 - 1
Userland/Libraries/LibCore/ConfigFile.h

@@ -52,7 +52,7 @@ public:
 
 
     bool is_dirty() const { return m_dirty; }
     bool is_dirty() const { return m_dirty; }
 
 
-    bool sync();
+    ErrorOr<void> sync();
 
 
     void remove_group(String const& group);
     void remove_group(String const& group);
     void remove_entry(String const& group, String const& key);
     void remove_entry(String const& group, String const& key);

+ 2 - 1
Userland/Services/AudioServer/Mixer.cpp

@@ -191,7 +191,8 @@ void Mixer::request_setting_sync()
         m_config_write_timer = Core::Timer::create_single_shot(
         m_config_write_timer = Core::Timer::create_single_shot(
             AUDIO_CONFIG_WRITE_INTERVAL,
             AUDIO_CONFIG_WRITE_INTERVAL,
             [this] {
             [this] {
-                m_config->sync();
+                if (auto result = m_config->sync(); result.is_error())
+                    dbgln("Failed to write audio mixer config: {}", result.error());
             },
             },
             this);
             this);
         m_config_write_timer->start();
         m_config_write_timer->start();

+ 5 - 1
Userland/Services/ConfigServer/ClientConnection.cpp

@@ -131,7 +131,11 @@ void ClientConnection::sync_dirty_domains_to_disk()
     dbgln("Syncing {} dirty domains to disk", dirty_domains.size());
     dbgln("Syncing {} dirty domains to disk", dirty_domains.size());
     for (auto domain : dirty_domains) {
     for (auto domain : dirty_domains) {
         auto& config = ensure_domain_config(domain);
         auto& config = ensure_domain_config(domain);
-        config.sync();
+        if (auto result = config.sync(); result.is_error()) {
+            dbgln("Failed to write config '{}' to disk: {}", domain, result.error());
+            // Put it back in the list since it's still dirty.
+            m_dirty_domains.set(domain);
+        }
     }
     }
 }
 }
 
 

+ 6 - 6
Userland/Services/WindowServer/Compositor.cpp

@@ -783,26 +783,26 @@ bool Compositor::set_background_color(const String& background_color)
 
 
     auto& wm = WindowManager::the();
     auto& wm = WindowManager::the();
     wm.config()->write_entry("Background", "Color", background_color);
     wm.config()->write_entry("Background", "Color", background_color);
-    bool ret_val = wm.config()->sync();
+    bool succeeded = !wm.config()->sync().is_error();
 
 
-    if (ret_val)
+    if (succeeded)
         Compositor::invalidate_screen();
         Compositor::invalidate_screen();
 
 
-    return ret_val;
+    return succeeded;
 }
 }
 
 
 bool Compositor::set_wallpaper_mode(const String& mode)
 bool Compositor::set_wallpaper_mode(const String& mode)
 {
 {
     auto& wm = WindowManager::the();
     auto& wm = WindowManager::the();
     wm.config()->write_entry("Background", "Mode", mode);
     wm.config()->write_entry("Background", "Mode", mode);
-    bool ret_val = wm.config()->sync();
+    bool succeeded = !wm.config()->sync().is_error();
 
 
-    if (ret_val) {
+    if (succeeded) {
         m_wallpaper_mode = mode_to_enum(mode);
         m_wallpaper_mode = mode_to_enum(mode);
         Compositor::invalidate_screen();
         Compositor::invalidate_screen();
     }
     }
 
 
-    return ret_val;
+    return succeeded;
 }
 }
 
 
 bool Compositor::set_wallpaper(RefPtr<Gfx::Bitmap> bitmap)
 bool Compositor::set_wallpaper(RefPtr<Gfx::Bitmap> bitmap)

+ 1 - 1
Userland/Services/WindowServer/ScreenLayout.ipp

@@ -271,7 +271,7 @@ bool ScreenLayout::save_config(Core::ConfigFile& config_file, bool sync) const
         config_file.remove_group(group_name);
         config_file.remove_group(group_name);
     }
     }
 
 
-    if (sync && !config_file.sync())
+    if (sync && config_file.sync().is_error())
         return false;
         return false;
     return true;
     return true;
 }
 }

+ 13 - 6
Userland/Services/WindowServer/WindowManager.cpp

@@ -254,7 +254,7 @@ bool WindowManager::apply_workspace_settings(unsigned rows, unsigned columns, bo
     if (save) {
     if (save) {
         m_config->write_num_entry("Workspace", "Rows", window_stack_rows());
         m_config->write_num_entry("Workspace", "Rows", window_stack_rows());
         m_config->write_num_entry("Workspace", "Columns", window_stack_columns());
         m_config->write_num_entry("Workspace", "Columns", window_stack_columns());
-        return m_config->sync();
+        return !m_config->sync().is_error();
     }
     }
     return true;
     return true;
 }
 }
@@ -264,7 +264,8 @@ void WindowManager::set_acceleration_factor(double factor)
     ScreenInput::the().set_acceleration_factor(factor);
     ScreenInput::the().set_acceleration_factor(factor);
     dbgln("Saving acceleration factor {} to config file at {}", factor, m_config->filename());
     dbgln("Saving acceleration factor {} to config file at {}", factor, m_config->filename());
     m_config->write_entry("Mouse", "AccelerationFactor", String::formatted("{}", factor));
     m_config->write_entry("Mouse", "AccelerationFactor", String::formatted("{}", factor));
-    m_config->sync();
+    if (auto result = m_config->sync(); result.is_error())
+        dbgln("Failed to save config file: {}", result.error());
 }
 }
 
 
 void WindowManager::set_scroll_step_size(unsigned step_size)
 void WindowManager::set_scroll_step_size(unsigned step_size)
@@ -272,7 +273,8 @@ void WindowManager::set_scroll_step_size(unsigned step_size)
     ScreenInput::the().set_scroll_step_size(step_size);
     ScreenInput::the().set_scroll_step_size(step_size);
     dbgln("Saving scroll step size {} to config file at {}", step_size, m_config->filename());
     dbgln("Saving scroll step size {} to config file at {}", step_size, m_config->filename());
     m_config->write_entry("Mouse", "ScrollStepSize", String::number(step_size));
     m_config->write_entry("Mouse", "ScrollStepSize", String::number(step_size));
-    m_config->sync();
+    if (auto result = m_config->sync(); result.is_error())
+        dbgln("Failed to save config file: {}", result.error());
 }
 }
 
 
 void WindowManager::set_double_click_speed(int speed)
 void WindowManager::set_double_click_speed(int speed)
@@ -281,7 +283,8 @@ void WindowManager::set_double_click_speed(int speed)
     m_double_click_speed = speed;
     m_double_click_speed = speed;
     dbgln("Saving double-click speed {} to config file at {}", speed, m_config->filename());
     dbgln("Saving double-click speed {} to config file at {}", speed, m_config->filename());
     m_config->write_entry("Input", "DoubleClickSpeed", String::number(speed));
     m_config->write_entry("Input", "DoubleClickSpeed", String::number(speed));
-    m_config->sync();
+    if (auto result = m_config->sync(); result.is_error())
+        dbgln("Failed to save config file: {}", result.error());
 }
 }
 
 
 int WindowManager::double_click_speed() const
 int WindowManager::double_click_speed() const
@@ -294,7 +297,8 @@ void WindowManager::set_buttons_switched(bool switched)
     m_buttons_switched = switched;
     m_buttons_switched = switched;
     dbgln("Saving mouse buttons switched state {} to config file at {}", switched, m_config->filename());
     dbgln("Saving mouse buttons switched state {} to config file at {}", switched, m_config->filename());
     m_config->write_bool_entry("Mouse", "ButtonsSwitched", switched);
     m_config->write_bool_entry("Mouse", "ButtonsSwitched", switched);
-    m_config->sync();
+    if (auto result = m_config->sync(); result.is_error())
+        dbgln("Failed to save config file: {}", result.error());
 }
 }
 
 
 bool WindowManager::get_buttons_switched() const
 bool WindowManager::get_buttons_switched() const
@@ -2080,7 +2084,10 @@ bool WindowManager::update_theme(String theme_path, String theme_name)
     m_palette = Gfx::PaletteImpl::create_with_anonymous_buffer(new_theme);
     m_palette = Gfx::PaletteImpl::create_with_anonymous_buffer(new_theme);
     m_config->write_entry("Theme", "Name", theme_name);
     m_config->write_entry("Theme", "Name", theme_name);
     m_config->remove_entry("Background", "Color");
     m_config->remove_entry("Background", "Color");
-    m_config->sync();
+    if (auto result = m_config->sync(); result.is_error()) {
+        dbgln("Failed to save config file: {}", result.error());
+        return false;
+    }
     invalidate_after_theme_or_font_change();
     invalidate_after_theme_or_font_change();
     return true;
     return true;
 }
 }

+ 1 - 1
Userland/Utilities/ini.cpp

@@ -35,7 +35,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
 
     if (value_to_write) {
     if (value_to_write) {
         config->write_entry(group, key, value_to_write);
         config->write_entry(group, key, value_to_write);
-        config->sync();
+        TRY(config->sync());
         return 0;
         return 0;
     }
     }
 
 

+ 2 - 2
Userland/Utilities/keymap.cpp

@@ -76,7 +76,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
 
         auto keymaps = String::join(',', mappings_vector);
         auto keymaps = String::join(',', mappings_vector);
         mapper_config->write_entry("Mapping", "Keymaps", keymaps);
         mapper_config->write_entry("Mapping", "Keymaps", keymaps);
-        mapper_config->sync();
+        TRY(mapper_config->sync());
         rc = set_keymap(mappings_vector.first());
         rc = set_keymap(mappings_vector.first());
         if (rc != 0) {
         if (rc != 0) {
             return rc;
             return rc;
@@ -90,7 +90,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         if (keymaps_vector.is_empty()) {
         if (keymaps_vector.is_empty()) {
             warnln("No keymaps configured - writing default configurations (en-us)");
             warnln("No keymaps configured - writing default configurations (en-us)");
             mapper_config->write_entry("Mapping", "Keymaps", "en-us");
             mapper_config->write_entry("Mapping", "Keymaps", "en-us");
-            mapper_config->sync();
+            TRY(mapper_config->sync());
             keymaps_vector.append("en-us");
             keymaps_vector.append("en-us");
         }
         }