Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Unblock paths > MAX_PATH without extended syntax. #3001

Merged
merged 1 commit into from
Aug 28, 2015

Conversation

JeremyKuhne
Copy link
Member

Also allow directories up to max component length (255) as opening does not
require extended syntax.

@stephentoub, @weshaggard, @Priya91

@JeremyKuhne JeremyKuhne force-pushed the RemoveMaxPathChecks branch 4 times, most recently from 146c01e to 4da74ba Compare August 28, 2015 04:49
@@ -9,10 +10,30 @@ partial class Interop
{
partial class mincore
{
/// <summary>
/// WARNING: This overload does not implicitly handle long paths.
/// </summary>
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename it then. It's strange to have two overloads so similar and yet with so different behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have any suggestions for naming? GetFullPathNameNoLongPath? This one in particular is used for the stack allocating code in PathHelper (all under MAX_PATH).

Like I mentioned, I intend to pass through these P/Invokes and normalize the naming. I want to drop the "W", use "Private" (anything better for that?), add comments on the privates to avoid leaking in other P/Invoke wrappers.

{
// There is no valid way to specify a relative path with two initial slashes
return !((path[1] == '\\') || (path[1] == '/'));
return !(IsDirectorySeparator(Path.DirectorySeparatorChar));
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be !IsDirectorySeparator(path[1])? It is probably worth making sure we have test coverage that would have caught this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah :/ I'll add the test for this one.

@weshaggard
Copy link
Member

Other then the one issue and the renaming which you will be doing in a follow-up. The change looks good to me.

Also allow directories up to max component length (255) as opening does not
require extended syntax.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants