Skip to content

bpo-38030: Fix os.stat failures on block devices on Windows #15681

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

Jump to bottom
Merged
merged 1 commit into from
Sep 4, 2019
Merged

bpo-38030: Fix os.stat failures on block devices on Windows #15681

merged 1 commit into from
Sep 4, 2019

Conversation

@zooba
Copy link
Member

@zooba zooba commented Sep 4, 2019 •
edited by bedevere-bot
Loading

@zooba zooba added needs backport to 3.8 OS-windows type-bug An unexpected behavior, bug, or error labels Sep 4, 2019
@zooba zooba merged commit 772ec0f into python:master Sep 4, 2019
@zooba zooba deleted the bpo-38030 branch September 4, 2019 21:42
@miss-islington
Copy link
Contributor

miss-islington commented Sep 4, 2019

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
@bedevere-bot
Copy link

bedevere-bot commented Sep 4, 2019

GH-15682 is a backport of this pull request to the 3.8 branch.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 4, 2019
@zooba @miss-islington
bpo-38030: Fix os.stat failures on block devices on Windows (pythonGH…
f8a221c 
…-15681) (cherry picked from commit 772ec0f) Co-authored-by: Steve Dower <steve.dower@python.org>
eryksun
eryksun reviewed Sep 4, 2019
View reviewed changes
Modules/posixmodule.c
error = retval ? GetLastError() : 0;
if (!CloseHandle(hFile)) {
retval = -1;
} else if (retval) {
Copy link
Contributor

@eryksun eryksun Sep 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Have you had a problem with this? The normal path for the internal implementation of CloseHandle is to call status = NtClose(handle). If the return status is successful, it just returns TRUE. Otherwise Windows sets the last error from the status code (i.e. RtlNtStatusToDosError, etc) and returns FALSE. It's not going to modify our thread's last error value if it succeeds.
Copy link
Member Author

@zooba zooba Sep 4, 2019

Choose a reason for hiding this comment

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

No, just being overly defensive. There are other functions that (incorrectly) change the last error, and it's easier to be consistent about preserving it than to rely on a not-strictly-followed convention.
Copy link
Member Author

@zooba zooba Sep 4, 2019

Choose a reason for hiding this comment

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

In particular, for others who come in and copy my code without necessarily checking severe edge cases like this :)
miss-islington added a commit that referenced this pull request Sep 4, 2019
@miss-islington @zooba
bpo-38030: Fix os.stat failures on block devices on Windows (GH-15681)
cad7abf 
(cherry picked from commit 772ec0f) Co-authored-by: Steve Dower <steve.dower@python.org>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@zooba @lisroach
bpo-38030: Fix os.stat failures on block devices on Windows (pythonGH…
647d99d 
…-15681)
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@zooba @DinoV
bpo-38030: Fix os.stat failures on block devices on Windows (pythonGH…
6f10922 
…-15681)
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
@zooba @websurfer5
bpo-38030: Fix os.stat failures on block devices on Windows (pythonGH…
b2d878a 
…-15681)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS-windows type-bug An unexpected behavior, bug, or error

5 participants

@zooba @miss-islington @bedevere-bot @eryksun @the-knights-who-say-ni