-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Conversation
/// </summary> | ||
internal static bool IsPathTooLong(string fullPath) | ||
{ | ||
return fullPath.Length >= Interop.libc.MaxPath; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is == considered too long? Is MaxPath defined to be one larger than the longest path allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see from reading further in the changes that we were already doing this comparison, so this isn't new. Is it correct though, on both Unix and Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe so (as .NET doesn't count the null terminator).
Other than a few minor comments and the Unix compilation failures (looks like you're missing a .cs file in one or more .csproj files), LGTM. cc: @ianhays |
@@ -21,7 +21,11 @@ internal static class IOInputs | |||
// Unix values vary system to system; just using really long values here likely to be more than on the average system | |||
public static readonly int MaxDirectory = 247; // Does not include trailing \0. This the maximum length that can be passed to APIs taking directory names, such as Directory.CreateDirectory, Directory.Move | |||
public static readonly int MaxPath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 259 : 10000; // Does not include trailing \0. | |||
public static readonly int MaxComponent = 255; | |||
public static readonly int MaxExtendedPath = short.MaxValue - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -1 is because of trailing \0 right? I'd suggest adding a comment like above that highlights this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as above. I'll add the comment.
3eb1b04
to
012857d
Compare
Add methods for checking max length as it varies on path format. Most code paths hit PathTooLong with GetFullPath- directories are a special case as Win32 apis have a special limit (248) for directory creation.
012857d
to
6f363e6
Compare
else | ||
{ | ||
// Will need to be updated with #2581 to allow all paths to MaxExtendedPath | ||
// minus legth of extended local or UNC prefix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: le_n_gth
Allow extended long path directories
YAY! |
Allow extended long path directories Commit migrated from dotnet/corefx@c6b0d8f
Add methods for checking max length as it varies on path format.
Most code paths hit PathTooLong with GetFullPath- directories are a special
case as Win32 apis have a special limit (248) for directory creation.
#2581, #2411, #2579
@stephentoub, @mmitche, @weshaggard