-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix path length 255 #21838
Fix path length 255 #21838
Conversation
src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.ReadDir.cs
Outdated
Show resolved
Hide resolved
But maybe it should? Wouldn't it be better to have NameBufferSize match Or are we absolutely sure this length is the maximum size on every platform we currently target and plan to target in the foreseeable future? |
|
Sure, I can do that. I see we already have SystemNative_GetReadDirRBufferSize. I still need to understand the GetMaxCharCount issue. |
|
Hmm, if the buffer size is not a constant anymore, this buffer in FileSystemEntry can no longer be I have to change to ArrayPool, or at least fall back to it. This change starts potentially modifying perf. Thoughts? |
|
I think I'm inclined to leave it hard coded at 255 + 1 and see whether this assert fires again - as it will if there is a system that has a larger limit and such a file or directory is encountered on it. From this it seems that may be true on ReiserFS, but then this assert may fail also: // dirent should be under 2k in size
assert(sizeof(struct dirent) < 2048);Edit: and according to Wikipedia/Stackoverflow: "ReiserFS shows 4032 bytes (but it is limited to 255 chars because of Linux VFS)." Incidentally NTFS is 512 bytes (256 UTF-16 code units). |
|
Ok, leave it hardcoded for now so that we can fix the original issue without it becoming a can of worms. It does make me uneasy, though. |
src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.ReadDir.cs
Show resolved
Hide resolved
src/System.Private.CoreLib/shared/Interop/Unix/System.Native/Interop.ReadDir.cs
Show resolved
Hide resolved
|
Unrelated build failures.
|
|
I see the ARM failures are on other jobs too. |
|
@dotnet-bot test Windows_NT x86 min_opt ryujit CoreCLR Perf Tests Correctness please |
|
@danmosemsft That's a well-known issue (https://github.com/dotnet/coreclr/issues/21843). We've started seeing it relatively recently. Hopefully dnceng can fix it soon. |
|
Thanks @BruceForstall |
|
@BruceForstall should I just rerun then? I don't see a timeline on https://github.com/dotnet/core-eng/issues/4555 |
|
@danmosemsft looks like other PRs around yours have completed successfully (though one or two have also failed). I'd just re-run. |
|
@dotnet-bot test Windows_NT arm64 Cross Debug Innerloop Build please |
|
I hope rerun helps; I don't know at this point how repeatable the failure is. |
|
ARM Ubuntu failure @BruceForstall |
|
1 gc test failure in ARM. Unrelated |
* Fix path length 255 * Fix assert * Dead code Signed-off-by: dotnet-bot <[email protected]>
* Fix path length 255 * Fix assert * Dead code Signed-off-by: dotnet-bot <[email protected]>
* Fix path length 255 * Fix assert * Dead code Signed-off-by: dotnet-bot <[email protected]>
* Fix path length 255 * Fix assert * Dead code Signed-off-by: dotnet-bot <[email protected]>
* Fix path length 255 * Fix assert * Dead code Signed-off-by: dotnet-bot <[email protected]>
* Fix path length 255 * Fix assert * Dead code Signed-off-by: dotnet-bot <[email protected]>
* Fix path length 255 * Fix assert * Dead code
|
FWIW, the dynamo issue is https://github.com/dotnet/coreclr/issues/17129 |
* Fix path length 255 * Fix assert * Dead code Commit migrated from dotnet/coreclr@c885564
Fix https://github.com/dotnet/corefx/issues/34359
Tests dotnet/corefx#34389
It's fortunate that the PAL did not need changing (SystemNative_ReadDirR) as well because then we would have a dependency in two directions.