Skip to content

More static analysis issues fixes #1129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vojtechtrefny merged 1 commit into from
Sep 24, 2025
Merged

More static analysis issues fixes #1129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 41 additions & 39 deletions src/plugins/fs/generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Comment on lines +714 to +716
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There's a potential NULL pointer dereference here. If bd_fs_unmount() returns FALSE but fails to set local_error (which would be a bug in bd_fs_unmount, but defensive coding is good practice), accessing local_error->message will cause a crash. It's safer to check if local_error is NULL before dereferencing it.

        g_set_error (error, BD_FS_ERROR, BD_FS_ERROR_UNMOUNT_FAIL,
                     "Failed to unmount '%s' after %s it: %s",
                     device, operation, local_error ? local_error->message : "Unknown error");

Copy link
Copy Markdown
Member

@tbzatek tbzatek Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BS! There's the code style and a certain trust in the code.

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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -829,23 +843,19 @@ 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
/* both info and unmount were unsuccessful but the 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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
11 changes: 1 addition & 10 deletions src/plugins/lvm/lvm-dbus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This is a good use of g_autofree. For consistency and to further simplify memory management, you might consider applying the same g_autofree pattern to the obj_path variable declared on the previous line. This would allow removing several manual g_free(obj_path) calls throughout the function.

guint64 log_task_id = 0;
guint64 prog_id = 0;
gdouble progress = 0.0;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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);
Comment thread
vojtechtrefny marked this conversation as resolved.

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) {
Expand Down