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.

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

bpo-45720: Drop references to shlwapi #29417

Merged
merged 1 commit into from
Nov 5, 2021
Merged

bpo-45720: Drop references to shlwapi #29417

merged 1 commit into from
Nov 5, 2021

Conversation

zooba
Copy link
Member

@zooba zooba commented Nov 5, 2021 •
edited by bedevere-bot
Loading

@zooba zooba requested a review from a team as a code owner November 5, 2021 01:06
@zooba
Copy link
Member Author

zooba commented Nov 5, 2021

Ping @eryksun (and yes, I've applied the same change to my other PR which will delete this code)

@zooba zooba merged commit a4774f4 into python:main Nov 5, 2021
@zooba zooba deleted the bpo-45720 branch November 5, 2021 23:06
@zooba zooba added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Nov 5, 2021
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

Sorry, @zooba, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a4774f42e35861c4bb16928cffb011c2d8a285ac 3.10

@miss-islington
Copy link
Contributor

Sorry @zooba, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker a4774f42e35861c4bb16928cffb011c2d8a285ac 3.9

eryksun
eryksun reviewed Nov 5, 2021
View reviewed changes
PC/getpathp.c
@@ -265,7 +264,8 @@ canonicalize(wchar_t *buffer, const wchar_t *path)
return _PyStatus_NO_MEMORY();
}

if (PathIsRelativeW(path)) {
const wchar_t *pathTail;
if (FAILED(PathCchSkipRoot(path, &pathTail)) || path == pathTail) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you not worried at all that canonicalize() will ever be called with paths that contain forward slashes? All of the PathCch* functions handle forward slash as just a name character, in which case skipping the root, combining with current directory, and getting a canonical path all misbehave badly. It wouldn't hurt to add a wcschr() call to check for forward slashes. Though another large buffer would have to be allocated on the stack in case path has to be copied to modify it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nah, it's shipped already and we'd have heard about issues if they exist. Plus #29041 is going to delete this code and replace it with code that handles both anyway.

zooba added a commit to zooba/cpython that referenced this pull request Nov 5, 2021
@zooba
bpo-45720: Drop references to shlwapi.dll on Windows (pythonGH-29417)
734aac3
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Nov 5, 2021
@bedevere-bot
Copy link

GH-29435 is a backport of this pull request to the 3.10 branch.

zooba added a commit that referenced this pull request Nov 6, 2021
@zooba
bpo-45720: Drop references to shlwapi.dll on Windows (GH-29417)
804ea41
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Nov 6, 2021
@bedevere-bot
Copy link

GH-29436 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 6, 2021
@zooba @miss-islington
bpo-45720: Drop references to shlwapi.dll on Windows (pythonGH-29417)
04f9b83 
(cherry picked from commit 804ea41)

Co-authored-by: Steve Dower <[email protected]>
zooba added a commit that referenced this pull request Nov 6, 2021
@miss-islington @zooba
bpo-45720: Drop references to shlwapi.dll on Windows (GH-29417)
5017306 
(cherry picked from commit 804ea41)

Co-authored-by: Steve Dower <[email protected]>

Co-authored-by: Steve Dower <[email protected]>
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
@zooba @remykarem
bpo-45720: Drop references to shlwapi.dll on Windows (pythonGH-29417)
47ed4a5
remykarem pushed a commit to remykarem/cpython that referenced this pull request Jan 30, 2022
@zooba @remykarem
bpo-45720: Drop references to shlwapi.dll on Windows (pythonGH-29417)
5370fdc
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

@zooba zooba

Labels
None yet
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