Skip to content

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

Merged
zooba merged 1 commit into
python:masterfrom
zooba:bpo-38030
Sep 4, 2019
Merged

bpo-38030: Fix os.stat failures on block devices on Windows#15681
zooba merged 1 commit into
python:masterfrom
zooba:bpo-38030

Conversation

@zooba

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

Copy link
Copy Markdown
Member

@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

miss-islington commented Sep 4, 2019

Copy link
Copy Markdown
Contributor

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

@bedevere-bot

bedevere-bot commented Sep 4, 2019

Copy link
Copy Markdown

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
Comment thread Modules/posixmodule.c
error = retval ? GetLastError() : 0;
if (!CloseHandle(hFile)) {
retval = -1;
} else if (retval) {

@eryksun eryksun Sep 4, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

@zooba zooba Sep 4, 2019

Copy link
Copy Markdown
Member Author

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.

@zooba zooba Sep 4, 2019

Copy link
Copy Markdown
Member Author

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

Reviewers

@eryksun eryksun eryksun left review comments

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

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