More static analysis issues fixes

[vojtechtrefny]

Second batch of issues found by static analysis -- unreachable code in LVM DBus and checked return in FS plugin.

[gemini-code-assist]

Summary of Changes

Hello @vojtechtrefny, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request continues the effort to resolve issues identified by static analysis, focusing on improving the robustness and maintainability of the codebase. It refactors error handling for file system unmount operations to ensure consistent behavior and better logging, and streamlines memory management within the LVM DBus plugin by adopting automatic resource deallocation, thereby removing redundant and potentially unreachable cleanup code.

Highlights

• FS Plugin Error Handling Improvement: Enhanced error handling in the File System plugin by checking the return value of g_rmdir when a temporary mountpoint cannot be removed after a failed mount operation, logging an informational message if the removal fails.
• FS Plugin Code Refactoring: Introduced a new static helper function fs_unmount to centralize the logic for unmounting a device and subsequently removing its temporary mountpoint. This function is now used consistently across various file system operations like resizing and getting info for XFS, NILFS2, and BTRFS, reducing code duplication and standardizing error propagation.
• LVM DBus Memory Management: Addressed unreachable code and simplified memory management in the LVM DBus plugin by utilizing g_autofree for the task_path variable. This change automatically handles the deallocation of task_path when it goes out of scope, eliminating the need for multiple explicit g_free calls and preventing potential memory leaks or double-free issues.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request addresses issues found by static analysis. The changes in src/plugins/lvm/lvm-dbus.c correctly leverage g_autofree to simplify memory management and prevent potential leaks, which is a good improvement. The changes in src/plugins/fs/generic.c introduce a helper function to reduce code duplication for unmounting and also add checks for the return value of g_rmdir. However, I've found a few issues with the implementation in fs/generic.c: the logic for checking the g_rmdir return value is incorrect in two places, and there is a critical memory leak and an incorrect return type in an error handling path within btrfs_get_info.

[vojtechtrefny]

/gemini review

[gemini-code-assist]

Code Review

This pull request fixes issues found by static analysis. In src/plugins/fs/generic.c, the return value of g_rmdir is now checked, and duplicated unmount logic has been refactored into a new fs_unmount helper function. In src/plugins/lvm/lvm-dbus.c, memory management is simplified by using g_autofree, which removes the need for manual g_free calls. These changes improve code correctness and maintainability. I have one suggestion to further improve clarity in the new helper function.

[vojtechtrefny]

/gemini review

[gemini-code-assist]

Code Review

This pull request addresses several issues found by static analysis, improving code robustness and maintainability. The changes in src/plugins/fs/generic.c correctly handle the return value of g_rmdir and refactor duplicated unmount logic into a new helper function, which is a good improvement. However, I've identified a potential null pointer dereference in this new function. In src/plugins/lvm/lvm-dbus.c, the use of g_autofree and the removal of unreachable code are positive changes. I've added a couple of suggestions to further improve the code quality in that file. Overall, these are valuable fixes.

[gemini-code-assist]

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");

[tbzatek]

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

[gemini-code-assist]

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.

[tbzatek]

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