Skip to content

bpo-45720: Drop references to shlwapi#29417

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

Merged
zooba merged 1 commit into from
Nov 5, 2021
Merged

bpo-45720: Drop references to shlwapi #29417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Windows/2021-11-05-01-05-46.bpo-45720.47Nc5I.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
Internal reference to :file:`shlwapi.dll` was dropped to help improve
startup time. This DLL will no longer be loaded at the start of every Python
process.
4 changes: 2 additions & 2 deletions PC/getpathp.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -92,7 +92,6 @@

#include <windows.h>
#include <pathcch.h>
#include <shlwapi.h>

#ifdef HAVE_SYS_TYPES_H
#include <sys/types.h>
Expand DownExpand Up@@ -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

@eryksuneryksunNov 5, 2021

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
MemberAuthor

@zoobazoobaNov 5, 2021

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.

wchar_t buff[MAXPATHLEN + 1];
if (!GetCurrentDirectoryW(MAXPATHLEN, buff)){
return _PyStatus_ERR("unable to find current working directory");
Expand Down
2 changes: 1 addition & 1 deletion PCbuild/_freeze_module.vcxproj
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -92,7 +92,7 @@
</ClCompile>
<Link>
<SubSystem>Console</SubSystem>
<AdditionalDependencies>version.lib;shlwapi.lib;ws2_32.lib;pathcch.lib;bcrypt.lib;%(AdditionalDependencies)</AdditionalDependencies>
<AdditionalDependencies>version.lib;ws2_32.lib;pathcch.lib;bcrypt.lib;%(AdditionalDependencies)</AdditionalDependencies>
</Link>
</ItemDefinitionGroup>
<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion PCbuild/pythoncore.vcxproj
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -106,7 +106,7 @@
<PreprocessorDefinitions Condition="$(IncludeExternals)">_Py_HAVE_ZLIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
</ClCompile>
<Link>
<AdditionalDependencies>version.lib;shlwapi.lib;ws2_32.lib;pathcch.lib;bcrypt.lib;%(AdditionalDependencies)</AdditionalDependencies>
<AdditionalDependencies>version.lib;ws2_32.lib;pathcch.lib;bcrypt.lib;%(AdditionalDependencies)</AdditionalDependencies>
</Link>
</ItemDefinitionGroup>
<ItemGroup>
Expand Down