-
Notifications
You must be signed in to change notification settings - Fork 30
fix: uses alternative method to open files on windows with shared permissions #136
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
Closed
MagnetoMallard
wants to merge
17
commits into
buu342:master
from
MagnetoMallard:windows-shared-file-fix
Closed
fix: uses alternative method to open files on windows with shared permissions #136
MagnetoMallard
wants to merge
17
commits into
buu342:master
from
MagnetoMallard:windows-shared-file-fix
Conversation
This file contains hidden or 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
… windows to allow for rom replacement. no idea if it will work, here goes a PR build.
…f MSVC is compiling this
… rather than tchar filename string
…ccording to my vibes based calcuation
… to use unicode. Weve done the conversion more simply this time
…ill locks the file :(
Author
|
I am closing this for now as it turns out that the inability for the rom file to be over-written was being caused by dangling file handles being left open by other parts of the code! So we don't need this. But if we ever do need to explicitly specify the handle on windows (in case Windows ever decides to get more strict about this, who knows), it can always be reopened! |
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.
Description
in windows, using the
fopenfunction locks the file for other programs. Therefore, the hot-reloading features would not work on windows. This behaviour is described in the documentation forfopen_s, but it can be assumed to apply tofopenas well. This is an attempt to fix that given the documentations recommendations to use _fsopen with explictly set shared file access permissions instead.This was tried at first, but it appeared to not be sufficient, as _fsopen does not have any permission set that allows files to be removed - you have to use lower level WinAPI functions to open files in a way that allows other applications to delete them.
Motivation and Context
As discussed on Discord!
How Has This Been Tested?
I am testing this now with the PR build, since I was struggling to set up the Windows build on my machine. will make further commits if it doesn't work!