Remove Interop.ReadDir.cs #34274
Remove Interop.ReadDir.cs #34274
Conversation
| int result = Interop.Sys.ReadDir(_directoryHandle, buffer, ref _entry); | ||
| switch (result) | ||
| if (_entryBuffer == null) | ||
| return; |
There was a problem hiding this comment.
Not sure about this. The old code used Span<byte>.Empty in case of null _entryBuffer. Then that empty span was used in ReadDir where it could crash with NullReferenceException because of ref MemoryMarshal.GetReference.
Perhaps we should throw some kind of an error here?
There was a problem hiding this comment.
Then that empty span was used in ReadDir where it could crash with NullReferenceException
Why do you think it could crash? ref MemoryMarshal.GetReference handles null just fine.
This if (_entryBuffer == null) return; should be deleted, and null check should be added to _entryBuffer.Length a few lines below.
Notice that when this method is called there is always MoveNext() on the stack that has a dummy fixed (byte* _ = _entryBuffer) to keep the buffer pinned. Ideally, the pointer to the buffer pinned by MoveNext() would be passed along and this would just use that pointer instead of doing another redundant fixed. Unfortunately, it cannot be done easily given how to code is factored.
There was a problem hiding this comment.
Oh, indeed! It doesn't throw NRE. I refactored a bit FindNextEntry for cases where it's possible to pass a pointer.
There was a problem hiding this comment.
I refactored a bit FindNextEntry for cases where it's possible to pass a pointer.
I am not sure whether this refactoring is an improvement. It would be an improvement if it is always possible to pass a pointer. Performance-wise, it is probably a tiny loss (the redundant fixed is very cheap).
src/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs
Outdated
Show resolved
Hide resolved
…numerator.Unix.cs Co-Authored-By: EgorBo <egorbo@gmail.com>
|
@dotnet-bot test Linux arm64 Release Build please |
|
Unrelated existing test issue |
* Remove Interop.ReadDir.cs * pass bufferPtr to FindNextEntry * Update src/System.IO.FileSystem/src/System/IO/Enumeration/FileSystemEnumerator.Unix.cs Co-Authored-By: EgorBo <egorbo@gmail.com> Commit migrated from dotnet/corefx@9158933
Since TimeZoneInfo was switched to FileSystem's
Interop.ReadDir.csin dotnet/coreclr#21622 we can now remove the oldInterop.ReadDir.csReadBufferSizeproperty was removed (see dotnet/coreclr#21622 (comment))/cc @jkotas