introduce a preserve command line switch in debugfs 'rdump' command #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
We should decide if we want to reformat the code or not. If there are ever new upstream changes in this tool, it will be hell to rebase to. If we are to take the risk, we should first reformat the code, and implement the new functionality on top, so that functional changes are not mixed with formatting ones on the same base. |
|
@vlaci this is an excellent point, I'll rework the MR without formatting stuff so we're safe if we have to rebase on upstream in the future. |
qkaiser
force-pushed
the
fix-debugfs
branch
3 times, most recently
from
ba671f1 to
5da34a8
Compare
|
This MR now only contains changes to the code itself and no reformatting. Code changes stick to tabs rather than spaces since it's what was used originally. |
orosam
reviewed
Feb 14, 2024
orosam
reviewed
Feb 14, 2024
rdump assumed the user wanted to preserve permissions and ownership when
dumping a filesystem directory recursively with 'rdump'. This is in
opposition with the way the 'dump' or 'dump_inode' command has been
designed, since it expose a '-p' command line switch to allow the end
users to explicitly opt-in for permission and ownership preservation.
The inability to explicitly ask for permission and ownership
preservation would get rdump to default to preservation, which is a
problem when faced with filesystems having directories with the read
flag but not the execute flag, since it would only allow to enumerate
the directory content, but not see the inode details. Therefore getting
debugfs in all kinds of issues trying to set ownership and permissions
of files it can't see.
This fix introduce a 'preserve' ('-p') flag in rdump command so that
users can explicitly opt-in for it, and debugfs will default to a safer
way of operation (no preserve).
Unused variable ext_written was reported by -Wunused-but-set-variable, so it's now guarded by the same preprocessor guards that protects its actual usage. Function without prototypes are note allowed in C23.
orosam
approved these changes
Feb 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
debugfsassumed the user wanted to preserve permissions and ownership when dumping a filesystem directory recursively withrdump. This is in opposition with the way thedump/dump_inodecommand has been designed, since it expose a-pcommand line switch to allow the end users to explicitly opt-in for permission and ownership preservation.The inability to explicitly ask for permission and ownership preservation would get rdump to default to preservation, which is a problem when faced with filesystems having directories with the read flag set but not the execute flag set, since it would only allow to enumerate the directory content, but not see the inode details. Therefore getting debugfs in all kinds of issues trying to set ownership and permissions of files it can't see.
This fix introduce a 'preserve' (
-p) flag in rdump command so that users can explicitly opt-in for it, and debugfs will default to a safer way of operation (no preserve).Note: I also fixed some code to get it to build with modern clang, and formatted the code with
clang-formatbecause the code is unreadable otherwise.