Skip to content

Commit 50877c5

Browse files
arrowdarrowd
committed
Make the InvalidSysctl exception more descriptive
The parameter being acted on by sysctl() call is determined by an arbitrarily sized array of integers called MIB. The current version of the code only prints the first element of the MIB, which isn't descriptive at all. Change it to print all elements of the MIB tuple. While there, make type signatures of low-level functions match their C versions. The problem was raised by a FreeBSD user: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=289022
1 parent 2b21c73 commit 50877c5

File tree

5 files changed

+57
-58
lines changed

5 files changed

+57
-58
lines changed

‎src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs‎

Lines changed: 50 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
using System.Runtime.InteropServices;
77
using System.Text;
88

9-
using size_t = System.IntPtr;
10-
119
// This implements shim for sysctl calls.
1210
// They are available on BSD systems - FreeBSD, OSX and others.
1311
// Linux has sysctl() but it is deprecated as well as it is missing sysctlbyname()
@@ -17,68 +15,69 @@ internal static partial class Interop
1715
internal static partial class Sys
1816
{
1917
[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_Sysctl", SetLastError = true)]
20-
private static unsafe partial int Sysctl(int* name, int namelen, void* value, size_t* len);
21-
22-
// This is 'raw' sysctl call, only wrapped to allocate memory if needed
23-
// caller always needs to free returned buffer using Marshal.FreeHGlobal()
18+
private static unsafe partial int Sysctl(int* name, uint namelen, void* value, nuint* len);
2419

25-
internal static unsafe void Sysctl(ReadOnlySpan<int> name, ref byte* value, ref int len)
20+
private static unsafe void Sysctl(ReadOnlySpan<int> name, ref byte* value, ref uint len)
2621
{
27-
fixed (int* ptr = &MemoryMarshal.GetReference(name))
22+
fixed (int* name_ptr = &MemoryMarshal.GetReference(name))
2823
{
29-
Sysctl(ptr, name.Length, ref value, ref len);
30-
}
31-
}
24+
uint name_len = (uint)name.Length;
25+
nuint bytesLength = len;
26+
int ret;
27+
bool autoSize = (value == null && len == 0);
3228

33-
private static unsafe void Sysctl(int* name, int name_len, ref byte* value, ref int len)
34-
{
35-
nint bytesLength = len;
36-
int ret = -1;
37-
bool autoSize = (value == null && len == 0);
38-
39-
if (autoSize)
40-
{
41-
// do one try to see how much data we need
42-
ret = Sysctl(name, name_len, value, &bytesLength);
43-
if (ret != 0)
29+
if (autoSize)
4430
{
45-
throw new InvalidOperationException(SR.Format(SR.InvalidSysctl, *name, Marshal.GetLastPInvokeError()));
31+
// do one try to see how much data we need
32+
ret = Sysctl(name, name_len, value, &bytesLength);
33+
if (ret != 0)
34+
{
35+
ThrowInvalidSysctlException(name, Marshal.GetLastPInvokeError());
36+
}
37+
value = (byte*)NativeMemory.Alloc(bytesLength);
4638
}
47-
value = (byte*)Marshal.AllocHGlobal((int)bytesLength);
48-
}
4939

50-
ret = Sysctl(name, name_len, value, &bytesLength);
51-
while (autoSize && ret != 0 && GetLastErrorInfo().Error == Error.ENOMEM)
52-
{
53-
// Do not use ReAllocHGlobal() here: we don't care about
54-
// previous contents, and proper checking of value returned
55-
// will make code more complex.
56-
Marshal.FreeHGlobal((IntPtr)value);
57-
if ((int)bytesLength == int.MaxValue)
58-
{
59-
throw new OutOfMemoryException();
60-
}
61-
if ((int)bytesLength >= int.MaxValue / 2)
62-
{
63-
bytesLength = int.MaxValue;
64-
}
65-
else
40+
ret = Sysctl(name, name_len, value, &bytesLength);
41+
while (autoSize && ret != 0 && GetLastErrorInfo().Error == Error.ENOMEM)
6642
{
67-
bytesLength = (int)bytesLength * 2;
43+
// Do not realloc here: we don't care about
44+
// previous contents, and proper checking of value returned
45+
// will make code more complex.
46+
NativeMemory.Free(value);
47+
if ((int)bytesLength == int.MaxValue)
48+
{
49+
throw new OutOfMemoryException();
50+
}
51+
if ((int)bytesLength >= int.MaxValue / 2)
52+
{
53+
bytesLength = int.MaxValue;
54+
}
55+
else
56+
{
57+
bytesLength *= 2;
58+
}
59+
value = (byte*)NativeMemory.Alloc(bytesLength);
60+
ret = Sysctl(name, name_len, value, &bytesLength);
6861
}
69-
value = (byte*)Marshal.AllocHGlobal(bytesLength);
70-
ret = Sysctl(name, name_len, value, &bytesLength);
71-
}
72-
if (ret != 0)
73-
{
74-
if (autoSize)
62+
if (ret != 0)
7563
{
76-
Marshal.FreeHGlobal((IntPtr)value);
64+
nint errno = Marshal.GetLastPInvokeError();
65+
if (autoSize)
66+
{
67+
NativeMemory.Free(value);
68+
}
69+
ThrowInvalidSysctlException(name, errno);
7770
}
78-
throw new InvalidOperationException(SR.Format(SR.InvalidSysctl, *name, Marshal.GetLastPInvokeError()));
71+
72+
len = (uint)bytesLength;
7973
}
74+
}
8075

81-
len = (int)bytesLength;
76+
static void ThrowInvalidSysctlException(ReadOnlySpan<int> name, nint errno)
77+
{
78+
throw new InvalidOperationException(SR.Format(SR.InvalidSysctl,
79+
string.Join(",", name.ToArray())
80+
errno));
8281
}
8382
}
8483
}

‎src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,14 @@ public unsafe struct @kinfo_proc
191191
[CTL_KERN, KERN_PROC, KERN_PROC_PID | (threads ? KERN_PROC_INC_THREAD : 0), pid]; // get specific process, possibly with threads
192192

193193
byte* pBuffer = null;
194-
int bytesLength = 0;
194+
uint bytesLength = 0;
195195
Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength);
196196

197197
kinfo_proc* kinfo = (kinfo_proc*)pBuffer;
198198

199199
Debug.Assert(kinfo->ki_structsize == sizeof(kinfo_proc));
200200

201-
count = (int)bytesLength / sizeof(kinfo_proc);
201+
count = (int)(bytesLength / sizeof(kinfo_proc));
202202

203203
// Buffer ownership transferred to the caller
204204
return kinfo;

‎src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,12 @@ public static unsafe string GetProcPath(int pid)
7979
{
8080
ReadOnlySpan<int> sysctlName = [CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, pid];
8181
byte* pBuffer = null;
82-
int bytesLength = 0;
82+
uint bytesLength = 0;
8383

8484
try
8585
{
8686
Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength);
87-
return System.Text.Encoding.UTF8.GetString(pBuffer, bytesLength - 1);
87+
return System.Text.Encoding.UTF8.GetString(pBuffer, (int)bytesLength - 1);
8888
}
8989
finally
9090
{

‎src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<?xml version="1.0" encoding="utf-8"?>
1+
<?xml version="1.0" encoding="utf-8"?>
22
<root>
33
<!--
44
Microsoft ResX Schema
@@ -331,7 +331,7 @@
331331
<value>Not all processes in process tree could be terminated.</value>
332332
</data>
333333
<data name="InvalidSysctl" xml:space="preserve">
334-
<value>sysctl {0} failed with {1} error.</value>
334+
<value>sysctl ({0}) failed with {1} error.</value>
335335
</data>
336336
<data name="InvalidPerfData" xml:space="preserve">
337337
<value>Invalid performance counter data with type '{0}'.</value>

‎src/libraries/System.Private.CoreLib/src/Resources/Strings.resx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3861,7 +3861,7 @@
38613861
<value>The returned enumerator does not implement IEnumVARIANT.</value>
38623862
</data>
38633863
<data name="InvalidSysctl" xml:space="preserve">
3864-
<value>sysctl {0} failed with {1} error.</value>
3864+
<value>sysctl ({0}) failed with {1} error.</value>
38653865
</data>
38663866
<data name="Argument_StructArrayTooLarge" xml:space="preserve">
38673867
<value>Array size exceeds addressing limitations.</value>

0 commit comments

Comments
 (0)