Skip to content
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.

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

refactor: use the term unmount instead of dismount #1477

Closed
wants to merge 4 commits into from

Conversation

tessus
Copy link
Contributor

@tessus tessus commented Jan 29, 2025

This PR has more than 1 commit on purpose and can be squashed during merge. (Or I can squash them as well, even though I like the commits separate in PRs (and not force push in a PR) to make discussion/comments easier to follow. But I will adhere to whatever process this project follows.)

  1. use UNMOUNT in constants
  2. use the term unmount in the GUI
  3. update the documentation
  4. add another command line argument/flag unmount (dismount will still work)

This is a rather extensive change in terms of LOC changed. I split it in several commits to make the review a bit easier. Although some whitespace errors were automatically fixed by the editor, for which I am sorry, since it might make a review a bit more tedious.

I haven't changed variable names for now, but can do so upon request. I have noticed that in certain parts of the code the term unmount had already been used.

This change updates the term DISMOUNT in constants to UNMOUNT.
Other occurrences (e.g. variable names) are left alone for now.
Additionally, the editor fixed a bunch of whitespace errors
automatically, thus a diff might not be as nice to read.
This change updates the GUI text and replaces dismount with unmount.
This change adds an argument 'unmount' for command line usage, while
trying to deprecate the old disnount argument.
The current dismount argument/flag will still work to not introduce
a breaking change.
Copy link
Member

@idrassi idrassi left a comment

Choose a reason for hiding this comment

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

The amount of white space changes is too much. This will cause problems when I will use my editor.
In addition of the issues I reported, can you please find a way to remove these white spaces? I cannot merge the PT with so many of them.

If it proves difficult, maybe a new PR without space changes will be simpler.

Thank you.

Comment on lines +4161 to +4162
ntStatus = TCFsctlCall (volumeFileObject, FSCTL_UNMOUNT_VOLUME, NULL, 0, NULL, 0);
Dump ("FSCTL_UNMOUNT_VOLUME returned %X\n", ntStatus);
Copy link
Member

Choose a reason for hiding this comment

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

regression: FSCTL_DISMOUNT_VOLUME is a Windows IOCTL that cannot be renamed.


if (NT_SUCCESS (ntStatus) || ntStatus == STATUS_VOLUME_DISMOUNTED)
if (NT_SUCCESS (ntStatus) || ntStatus == STATUS_VOLUME_UNMOUNTED)
Copy link
Member

Choose a reason for hiding this comment

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

STATUS_VOLUME_DISMOUNTED is a Windows error code that cannot be renamed

@@ -9674,7 +9674,7 @@ HANDLE DismountDrive (wchar_t *devName, wchar_t *devicePath)

attempt = UNMOUNT_MAX_AUTO_RETRIES;

while (!(bResult = DeviceIoControl (hVolume, FSCTL_DISMOUNT_VOLUME, NULL, 0, NULL, 0, &dwResult, NULL))
while (!(bResult = DeviceIoControl (hVolume, FSCTL_UNMOUNT_VOLUME, NULL, 0, NULL, 0, &dwResult, NULL))
Copy link
Member

Choose a reason for hiding this comment

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

FSCTL_DISMOUNT_VOLUME is a Windows IOCTL that cannot be renamed

@@ -2038,7 +2038,7 @@ static int DismountFileSystem (HWND hwndDlg, HANDLE dev,

attempt = UNMOUNT_MAX_AUTO_RETRIES * 10;

while (!(bResult = DeviceIoControl (dev, FSCTL_DISMOUNT_VOLUME, NULL, 0, NULL, 0, &dwResult, NULL))
while (!(bResult = DeviceIoControl (dev, FSCTL_UNMOUNT_VOLUME, NULL, 0, NULL, 0, &dwResult, NULL))
Copy link
Member

Choose a reason for hiding this comment

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

FSCTL_DISMOUNT_VOLUME is a Windows IOCTL that cannot be renamed.

@tessus
Copy link
Contributor Author

tessus commented Jan 29, 2025

Thank you for the review and pointing out my oversight on the WinAPI calls.
I haven't touched Windows since Windws 3.11 or NT server 4.0.

The amount of white space changes is too much. This will cause problems when I will use my editor.

Just a suggestion for the future. You might want to use a code formatting tool. The current code is quite inconsistent: there's a mix of tabs and spaces, different coding styles, whitspace at the end of lines and in the beginning of empty lines.
It would certainly help you a lot. I am impressed that you don't get mad when working on the code. ;-)

I used UniversalIndentGUI in the past. It's cross platform and it allows for different engines. You can use predefined styles, modify an existing one, or create a new style from scratch.

I am not sure why removing whitespaces at the end of lines or at the beginning of empty lines would mess up your editor, but you are the boss. I'll get on it.

In addition of the issues I reported, can you please find a way to remove these white spaces? I cannot merge the PT with so many of them.
If it proves difficult, maybe a new PR without space changes will be simpler.

I will have to create a new PR, reverting the whitspace changes one at a time manually would take forever.
First I have to find a way to tell my editor not to fix whitespace errors. This might take the longest for me to figure out.

We can keep this PR open for now or close it. It's up to you.

@tessus
Copy link
Contributor Author

tessus commented Jan 30, 2025

superseded by #1478

@tessus tessus closed this Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants