Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit e82401b

Browse files
nguerreranguerrera
committed
Respond to PR feeback
* Defer conversions until necessary * Rename Errno -> RawErrno * Return null for ERANGE for strerror * Add conversion helper for Error -> ErrorInfo * Fix typos * Remove unnecessary _POSIX_C_SOURCE define * Fix names and sizes of managed FileStatus members * Don't double-initialize ErrorInfo out param * Zero out BirthTime when not available * Put back newline at EOL * Restore size of UnixFileSystemObject * Use ignored local name for ignored error info * Rename Interop.System to Interop.Sys to avoid collision with System namespace
1 parent 2b01345 commit e82401b

File tree

27 files changed

+168
-128
lines changed

27 files changed

+168
-128
lines changed

‎src/Common/src/Interop/Linux/libc/Interop.inotify.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal static int inotify_rm_watch(int fd, int wd)
2222
int result = Interop.libc.inotify_rm_watch_extern(fd, wd);
2323
if (result < 0)
2424
{
25-
Error hr = Interop.System.GetLastError();
25+
Error hr = Interop.Sys.GetLastError();
2626
if (hr == Interop.Error.EINVAL)
2727
{
2828
// This specific case means that there was a deleted event in the queue that was not processed
@@ -32,7 +32,7 @@ internal static int inotify_rm_watch(int fd, int wd)
3232
}
3333
else
3434
{
35-
global::System.Diagnostics.Debug.Fail("inotify_rm_watch failed with " + hr);
35+
System.Diagnostics.Debug.Fail("inotify_rm_watch failed with " + hr);
3636
}
3737
}
3838

‎src/Common/src/Interop/OSX/Interop.libproc.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ internal static unsafe string proc_pidpath(int pid)
441441
}
442442

443443
// OS X uses UTF-8. The conversion may not strip off all trailing \0s so remove them here
444-
return global::System.Text.Encoding.UTF8.GetString(pBuffer, result);
444+
return System.Text.Encoding.UTF8.GetString(pBuffer, result);
445445
}
446446

447447
/// <summary>

‎src/Common/src/Interop/Unix/Interop.Errors.cs

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ internal enum Error
1111
{
1212
// These values were defined in src/Native/System.Native/fxerrno.h
1313
//
14-
// They compare against values obtaied via Interop.System.GetLastError() not Marshal.GetLastWin32Error()
14+
// They compare against values obtaied via Interop.Sys.GetLastError() not Marshal.GetLastWin32Error()
1515
// which obtains the raw errno that varies between unixes. The strong typing as an enum is meant to
1616
// prevent confusing the two. Casting to or from int is suspect. Use GetLastErrorInfo() if you need to
17-
// correlate these to the underlying platform values or obtain the correspodning error message.
17+
// correlate these to the underlying platform values or obtain the corresponding error message.
1818
//
1919

20+
SUCCESS = 0,
21+
2022
E2BIG = 0x10001, // Argument list too long.
2123
EACCES = 0x10002, // Permission denied.
2224
EADDRINUSE = 0x10003, // Address in use.
@@ -104,37 +106,42 @@ internal enum Error
104106
EWOULDBLOCK = EAGAIN, // Operation would block
105107
}
106108

109+
107110
// Represents a platform-agnostic Error and underlying platform-specific errno
108111
internal struct ErrorInfo
109112
{
110-
internal readonly Error Error;
111-
internal readonly int Errno;
113+
private Error _error;
114+
private int _rawErrno;
115+
116+
internal ErrorInfo(int errno)
117+
{
118+
_error = (Error)(-1);
119+
_rawErrno = errno;
120+
}
112121

113122
internal ErrorInfo(Error error)
114123
{
115-
Error = error;
116-
Errno = Interop.System.ConvertErrorPalToPlatform(error);
124+
_error = error;
125+
_rawErrno = -1;
117126
}
118127

119-
internal ErrorInfo(int errno)
128+
internal Error Error
120129
{
121-
Error = Interop.System.ConvertErrorPlatformToPal(errno);
122-
Errno = errno;
130+
get { return _error == (Error)(-1) ? (_error = Interop.Sys.ConvertErrorPlatformToPal(_rawErrno)) : _error; }
123131
}
124132

125-
internal ErrorInfo(Error error, int errno)
133+
internal int RawErrno
126134
{
127-
Error = error;
128-
Errno = errno;
135+
get { return _rawErrno == -1 ? (_rawErrno = Interop.Sys.ConvertErrorPalToPlatform(_error)) : _rawErrno; }
129136
}
130137

131138
internal string GetErrorMessage()
132139
{
133-
return Interop.System.StrError(Errno);
140+
return Interop.Sys.StrError(RawErrno);
134141
}
135142
}
136143

137-
internal partial class System
144+
internal partial class Sys
138145
{
139146
internal static Error GetLastError()
140147
{
@@ -148,10 +155,21 @@ internal static ErrorInfo GetLastErrorInfo()
148155

149156
internal static unsafe string StrError(int platformErrno)
150157
{
151-
const int maxErrorMessageLength = 1024; // length long enough for most any Unix error
152-
byte* buffer = stackalloc byte[maxErrorMessageLength];
153-
IntPtr message = StrErrorR(platformErrno, buffer, maxErrorMessageLength);
154-
return Marshal.PtrToStringAnsi(message);
158+
int maxBufferLength = 1024; // should be long enough for most any UNIX error
159+
byte* buffer = stackalloc byte[maxBufferLength];
160+
byte* message = StrErrorR(platformErrno, buffer, maxBufferLength);
161+
162+
if (message == null)
163+
{
164+
// This means the buffer was not large enough, but still contains
165+
// as much of the error message as possible and is guaranteed to
166+
// be null-terminated. We're not currently resizing/retrying because
167+
// maxBufferLength is large enough in practice, but we could do
168+
// so here in the future if necessary.
169+
message = buffer;
170+
}
171+
172+
return Marshal.PtrToStringAnsi((IntPtr)message);
155173
}
156174

157175
[DllImport(Libraries.SystemNative)]
@@ -161,6 +179,22 @@ internal static unsafe string StrError(int platformErrno)
161179
internal static extern int ConvertErrorPalToPlatform(Error error);
162180

163181
[DllImport(Libraries.SystemNative)]
164-
private static unsafe extern IntPtr StrErrorR(int platformErrno, byte* buffer, int bufferSize);
182+
private static unsafe extern byte* StrErrorR(int platformErrno, byte* buffer, int bufferSize);
183+
}
184+
}
185+
186+
// NOTE: extension method can't be nested inside Interop class.
187+
internal static class InteropErrorExtensions
188+
{
189+
// Intended usage is e.g. Interop.Error.EFAIL.Info() for brevity
190+
// vs. new Interop.ErrorInfo(Interop.Error.EFAIL) for synthesizing
191+
// errors. Errors originated from the system should be obtained
192+
// via GetLastErrorInfo(), not GetLastError().Info() as that will
193+
// convert twice, which is not only inefficient but also lossy if
194+
// we ever encounter a raw errno that no equivalent in the Error
195+
// enum.
196+
public static Interop.ErrorInfo Info(this Interop.Error error)
197+
{
198+
return new Interop.ErrorInfo(error);
165199
}
166200
}

‎src/Common/src/Interop/Unix/Interop.IOErrors.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal static bool CheckIo(long result, string path = null, bool isDirectory =
2626
{
2727
if (result < 0)
2828
{
29-
ErrorInfo errorInfo = Interop.System.GetLastErrorInfo();
29+
ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
3030
if (errorRewriter != null)
3131
{
3232
errorInfo = errorRewriter(errorInfo);
@@ -90,8 +90,8 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string pat
9090

9191
case Error.EWOULDBLOCK:
9292
return !string.IsNullOrEmpty(path) ?
93-
new IOException(SR.Format(SR.IO_SharingViolation_File, path), errorInfo.Errno) :
94-
new IOException(SR.IO_SharingViolation_NoFileName, errorInfo.Errno);
93+
new IOException(SR.Format(SR.IO_SharingViolation_File, path), errorInfo.RawErrno) :
94+
new IOException(SR.IO_SharingViolation_NoFileName, errorInfo.RawErrno);
9595

9696
case Error.ECANCELED:
9797
return new OperationCanceledException();
@@ -102,7 +102,7 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string pat
102102
case Error.EEXIST:
103103
if (!string.IsNullOrEmpty(path))
104104
{
105-
return new IOException(SR.Format(SR.IO_FileExists_Name, path), errorInfo.Errno);
105+
return new IOException(SR.Format(SR.IO_FileExists_Name, path), errorInfo.RawErrno);
106106
}
107107
goto default;
108108

@@ -113,6 +113,6 @@ internal static Exception GetExceptionForIoErrno(ErrorInfo errorInfo, string pat
113113

114114
internal static Exception GetIOException(Interop.ErrorInfo errorInfo)
115115
{
116-
return new IOException(errorInfo.GetErrorMessage(), errorInfo.Errno);
116+
return new IOException(errorInfo.GetErrorMessage(), errorInfo.RawErrno);
117117
}
118118
}

‎src/Common/src/Interop/Unix/System.Native/Interop.Stat.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,19 @@
66

77
internal static partial class Interop
88
{
9-
internal static partial class System
9+
internal static partial class Sys
1010
{
1111
internal struct FileStatus
1212
{
1313
internal FileStatusFlags Flags;
1414
internal int Mode;
1515
internal int Uid;
1616
internal int Gid;
17-
internal int Size;
18-
internal int AccessTime;
19-
internal int ModificationTime;
20-
internal int StatusChangeTime;
21-
internal int CreationTime;
17+
internal long Size;
18+
internal long ATime;
19+
internal long MTime;
20+
internal long CTime;
21+
internal long BirthTime;
2222
}
2323

2424
internal static class FileTypes

‎src/Common/src/Interop/Unix/libc/Interop.ResourceLimits.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ internal static rlimit getrlimit(RLIMIT_Resources resource)
6666
int result = getrlimit(resource, ref info);
6767
if (result < 0)
6868
{
69-
throw new global::System.ComponentModel.Win32Exception(SR.ResourceLimitQueryFailure);
69+
throw new System.ComponentModel.Win32Exception(SR.ResourceLimitQueryFailure);
7070
}
7171

7272
return info;

‎src/Common/src/Interop/Unix/libc/Interop.getcwd.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private static unsafe string getcwd(byte* ptr, int bufferSize)
6161

6262
// Otherwise, if it failed due to the buffer being too small, return null;
6363
// for anything else, throw.
64-
ErrorInfo errorInfo = Interop.System.GetLastErrorInfo();
64+
ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
6565
if (errorInfo.Error == Interop.Error.ERANGE)
6666
{
6767
return null;

‎src/Common/src/Interop/Unix/libc/Interop.getsetpriority.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ internal static int getpriority(PriorityWhich which, int who, out int priority)
4848
[DllImport(Libraries.Libc, SetLastError = true)]
4949
internal static extern int setpriority(PriorityWhich which, int who, int prio);
5050

51-
internal static global::System.Diagnostics.ThreadPriorityLevel GetThreadPriorityFromNiceValue(int nice)
51+
internal static System.Diagnostics.ThreadPriorityLevel GetThreadPriorityFromNiceValue(int nice)
5252
{
5353
Debug.Assert((nice >= -20) && (nice <= 20));
5454
return
@@ -61,7 +61,7 @@ internal static int getpriority(PriorityWhich which, int who, out int priority)
6161
ThreadPriorityLevel.Idle;
6262
}
6363

64-
internal static int GetNiceValueFromThreadPriority(global::System.Diagnostics.ThreadPriorityLevel priority)
64+
internal static int GetNiceValueFromThreadPriority(System.Diagnostics.ThreadPriorityLevel priority)
6565
{
6666
return (priority == ThreadPriorityLevel.TimeCritical ? -20 :
6767
priority == ThreadPriorityLevel.Highest ? -15 :

‎src/Common/src/Interop/Unix/libc/Interop.mountpoints.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,13 @@ internal static partial class libc
3939
internal static bool TryGetStatFsForDriveName(string name, out statfs data, out ErrorInfo errorInfo)
4040
{
4141
data = default(statfs);
42-
errorInfo = default(ErrorInfo);
4342
if (get_statfs(name, out data) < 0)
4443
{
45-
errorInfo = Interop.System.GetLastErrorInfo();
44+
errorInfo = Interop.Sys.GetLastErrorInfo();
4645
return false;
4746
}
47+
48+
errorInfo = default(ErrorInfo);
4849
return true;
4950
}
5051

@@ -60,7 +61,7 @@ internal static statfs GetStatFsForDriveName(string name)
6061
if (!TryGetStatFsForDriveName(name, out data, out errorInfo))
6162
{
6263
throw errorInfo.Error == Error.ENOENT ?
63-
new global::System.IO.DriveNotFoundException(SR.Format(SR.IO_DriveNotFound_Drive, name)) : // match Win32 exception
64+
new System.IO.DriveNotFoundException(SR.Format(SR.IO_DriveNotFound_Drive, name)) : // match Win32 exception
6465
GetExceptionForIoErrno(errorInfo, isDirectory: true);
6566
}
6667
return data;

‎src/Common/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ internal static SafeFileHandle Open(string path, Interop.libc.OpenFlags flags, i
4444
// Open the file.
4545
int fd;
4646
while (Interop.CheckIo(fd = Interop.libc.open(path, flags, mode), path, isDirectory: enoentDueToDirectory,
47-
errorRewriter: e => (e.Error == Interop.Error.EISDIR) ? new Interop.ErrorInfo(Interop.Error.EACCES) : e)) ;
47+
errorRewriter: e => (e.Error == Interop.Error.EISDIR) ? Interop.Error.EACCES.Info() : e)) ;
4848
Debug.Assert(fd >= 0);
4949
handle.SetHandle((IntPtr)fd);
5050
Debug.Assert(!handle.IsInvalid);
@@ -55,12 +55,12 @@ internal static SafeFileHandle Open(string path, Interop.libc.OpenFlags flags, i
5555
if (Interop.libcoreclr.GetFileInformationFromFd(fd, out buf) != 0)
5656
{
5757
handle.Dispose();
58-
throw Interop.GetExceptionForIoErrno(Interop.System.GetLastErrorInfo(), path);
58+
throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), path);
5959
}
6060
if ((buf.mode & Interop.libcoreclr.FileTypes.S_IFMT) == Interop.libcoreclr.FileTypes.S_IFDIR)
6161
{
6262
handle.Dispose();
63-
throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(Interop.Error.EACCES), path, isDirectory: true);
63+
throw Interop.GetExceptionForIoErrno(Interop.Error.EACCES.Info(), path, isDirectory: true);
6464
}
6565
}
6666
return handle;

0 commit comments

Comments
 (0)