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.

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
merged 1 commit into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff 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 Down Expand 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

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.

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
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff 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