From 6489761101e4693091e2c6b80d33843cf4eaaa0a Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 24 Sep 2025 12:56:42 +0200 Subject: [PATCH] More static analysis issues fixes Second batch of issues found by static analysis -- unreachable code in LVM DBus and checked return in FS plugin. --- src/plugins/fs/generic.c | 80 +++++++++++++++++++------------------- src/plugins/lvm/lvm-dbus.c | 11 +----- 2 files changed, 42 insertions(+), 49 deletions(-) diff --git a/src/plugins/fs/generic.c b/src/plugins/fs/generic.c index 4a0c7cf8c..0fc2a3093 100644 --- a/src/plugins/fs/generic.c +++ b/src/plugins/fs/generic.c @@ -686,7 +686,9 @@ static gchar* fs_mount (const gchar *device, gchar *fstype, gboolean read_only, ret = bd_fs_mount (device, mountpoint, fstype, read_only ? "nosuid,nodev,ro" : "nosuid,nodev", NULL, &l_error); if (!ret) { g_propagate_prefixed_error (error, l_error, "Failed to mount '%s': ", device); - g_rmdir (mountpoint); + if (g_rmdir (mountpoint) != 0) + bd_utils_log_format (BD_UTILS_LOG_INFO, "Failed to remove temporary mountpoint '%s'", + mountpoint); g_free (mountpoint); return NULL; } else @@ -703,6 +705,26 @@ static gchar* fs_mount (const gchar *device, gchar *fstype, gboolean read_only, return mountpoint; } +static gboolean fs_unmount (const gchar *device, const gchar *mountpoint, const gchar *operation, GError **error) { + gboolean ret = FALSE; + GError *local_error = NULL; + + ret = bd_fs_unmount (mountpoint, FALSE, FALSE, NULL, &local_error); + if (!ret) { + g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL, + "Failed to unmount '%s' after %s it: %s", + device, operation, local_error->message); + g_clear_error (&local_error); + return FALSE; + } else { + if (g_rmdir (mountpoint) != 0) + bd_utils_log_format (BD_UTILS_LOG_INFO, "Failed to remove temporary mountpoint '%s'", + mountpoint); + } + return TRUE; +} + + /** * xfs_resize_device: * @device: the device the file system of which to resize @@ -739,22 +761,18 @@ static gboolean xfs_resize_device (const gchar *device, guint64 new_size, const success = bd_fs_xfs_resize (mountpoint, new_size, extra, error); if (unmount) { - ret = bd_fs_unmount (mountpoint, FALSE, FALSE, NULL, &local_error); + ret = fs_unmount (device, mountpoint, "resizing", &local_error); if (!ret) { if (success) { /* resize was successful but unmount failed */ - g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL, - "Failed to unmount '%s' after resizing it: %s", - device, local_error->message); - g_clear_error (&local_error); + g_propagate_error (error, local_error); return FALSE; } else /* both resize and unmount were unsuccessful but the error from the resize is more important so just ignore the unmount error */ g_clear_error (&local_error); - } else - g_rmdir (mountpoint); + } } return success; @@ -794,22 +812,18 @@ static gboolean nilfs2_resize_device (const gchar *device, guint64 new_size, GEr success = bd_fs_nilfs2_resize (device, new_size, error); if (unmount) { - ret = bd_fs_unmount (mountpoint, FALSE, FALSE, NULL, &local_error); + ret = fs_unmount (device, mountpoint, "resizing", &local_error); if (!ret) { if (success) { /* resize was successful but unmount failed */ - g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL, - "Failed to unmount '%s' after resizing it: %s", - device, local_error->message); - g_clear_error (&local_error); + g_propagate_error (error, local_error); return FALSE; } else /* both resize and unmount were unsuccessful but the error from the resize is more important so just ignore the unmount error */ g_clear_error (&local_error); - } else - g_rmdir (mountpoint); + } } return success; @@ -829,14 +843,11 @@ static BDFSBtrfsInfo* btrfs_get_info (const gchar *device, GError **error) { btrfs_info = bd_fs_btrfs_get_info (mountpoint, error); if (unmount) { - ret = bd_fs_unmount (mountpoint, FALSE, FALSE, NULL, &local_error); + ret = fs_unmount (device, mountpoint, "getting info", &local_error); if (!ret) { if (btrfs_info) { /* info was successful but unmount failed */ - g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL, - "Failed to unmount '%s' after getting info: %s", - device, local_error->message); - g_clear_error (&local_error); + g_propagate_error (error, local_error); bd_fs_btrfs_info_free (btrfs_info); return NULL; } else @@ -844,8 +855,7 @@ static BDFSBtrfsInfo* btrfs_get_info (const gchar *device, GError **error) { from the info is more important so just ignore the unmount error */ g_clear_error (&local_error); - } else - g_rmdir (mountpoint); + } } return btrfs_info; @@ -865,22 +875,18 @@ static gboolean btrfs_resize_device (const gchar *device, guint64 new_size, GErr success = bd_fs_btrfs_resize (mountpoint, new_size, NULL, error); if (unmount) { - ret = bd_fs_unmount (mountpoint, FALSE, FALSE, NULL, &local_error); + ret = fs_unmount (device, mountpoint, "resizing", &local_error); if (!ret) { if (success) { /* resize was successful but unmount failed */ - g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL, - "Failed to unmount '%s' after resizing it: %s", - device, local_error->message); - g_clear_error (&local_error); + g_propagate_error (error, local_error); return FALSE; } else /* both resize and unmount were unsuccessful but the error from the resize is more important so just ignore the unmount error */ g_clear_error (&local_error); - } else - g_rmdir (mountpoint); + } } return success; @@ -900,22 +906,18 @@ static gboolean btrfs_set_label (const gchar *device, const gchar *label, GError success = bd_fs_btrfs_set_label (mountpoint, label, error); if (unmount) { - ret = bd_fs_unmount (mountpoint, FALSE, FALSE, NULL, &local_error); + ret = fs_unmount (device, mountpoint, "setting label", &local_error); if (!ret) { if (success) { - /* resize was successful but unmount failed */ - g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL, - "Failed to unmount '%s' after setting label: %s", - device, local_error->message); - g_clear_error (&local_error); + /* label was successful but unmount failed */ + g_propagate_error (error, local_error); return FALSE; } else - /* both set label and unmount were unsuccessful but the error - from the set label is more important so just ignore the + /* both label and unmount were unsuccessful but the error + from the label is more important so just ignore the unmount error */ g_clear_error (&local_error); - } else - g_rmdir (mountpoint); + } } return success; diff --git a/src/plugins/lvm/lvm-dbus.c b/src/plugins/lvm/lvm-dbus.c index ad7752c60..197dcd123 100644 --- a/src/plugins/lvm/lvm-dbus.c +++ b/src/plugins/lvm/lvm-dbus.c @@ -693,7 +693,7 @@ static GVariant* call_lvm_method (const gchar *obj, const gchar *intf, const gch static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const gchar *method, GVariant *params, GVariant *extra_params, const BDExtraArg **extra_args, gboolean lock_config, GError **error) { GVariant *ret = NULL; gchar *obj_path = NULL; - gchar *task_path = NULL; + g_autofree gchar *task_path = NULL; guint64 log_task_id = 0; guint64 prog_id = 0; gdouble progress = 0.0; @@ -726,7 +726,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const g_free (log_msg); /* got a valid result, just return */ g_variant_unref (ret); - g_free (task_path); g_free (obj_path); bd_utils_report_finished (prog_id, "Completed"); return TRUE; @@ -740,7 +739,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const method, obj, log_msg); bd_utils_log_task_status (log_task_id, log_msg); bd_utils_report_finished (prog_id, log_msg); - g_free (task_path); g_free (log_msg); return FALSE; } @@ -751,7 +749,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const g_variant_unref (ret); } else { bd_utils_log_task_status (log_task_id, "No result, no job started"); - g_free (task_path); bd_utils_report_finished (prog_id, "Completed"); g_variant_unref (ret); return TRUE; @@ -808,7 +805,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const method, obj); bd_utils_report_finished (prog_id, l_error->message); g_propagate_error (error, l_error); - g_free (task_path); return FALSE; } else { g_variant_get (ret, "o", &obj_path); @@ -837,7 +833,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const "Got unknown error when running '%s' method on the '%s' object.", method, obj); } - g_free (task_path); g_propagate_error (error, l_error); return FALSE; } else @@ -852,7 +847,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const if (ret) g_variant_unref (ret); - g_free (task_path); return TRUE; } } else { @@ -863,9 +857,6 @@ static gboolean call_lvm_method_sync (const gchar *obj, const gchar *intf, const bd_utils_report_finished (prog_id, "Completed"); return FALSE; } - g_free (task_path); - - return TRUE; } static gboolean call_lvm_obj_method_sync (const gchar *obj_id, const gchar *intf, const gchar *method, GVariant *params, GVariant *extra_params, const BDExtraArg **extra_args, gboolean lock_config, GError **error) {