Free allocated buffer after UTF8 encode on FreeBSD (#43431)

* Free allocated buffer after UTF8 encode on FreeBSD

* Address PR feedback

diff --git a/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs b/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs
index cb180fa..f524faa 100644 (file)
--- a/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs
+++ b/src/libraries/Common/src/Interop/BSD/System.Native/Interop.Sysctl.cs
@@ -23,46 +23,37 @@ internal static partial class Interop
         // This is 'raw' sysctl call, only wrapped to allocate memory if needed
         // caller always needs to free returned buffer using  Marshal.FreeHGlobal()
 
-        public static unsafe int Sysctl(Span<int> name, ref byte* value, ref int len)
+        internal static unsafe void Sysctl(Span<int> name, ref byte* value, ref int len)
         {
-            fixed (int * ptr = &MemoryMarshal.GetReference(name))
+            fixed (int* ptr = &MemoryMarshal.GetReference(name))
             {
-                return Sysctl(ptr, name.Length, ref value, ref len);
+                Sysctl(ptr, name.Length, ref value, ref len);
             }
         }
 
-        public static unsafe int Sysctl(int* name, int name_len, ref byte* value, ref int len)
+        private static unsafe void Sysctl(int* name, int name_len, ref byte* value, ref int len)
         {
             IntPtr bytesLength = (IntPtr)len;
-            byte * pBuffer = value;
-            value = null;
-            int ret=-1;
+            int ret = -1;
 
             if (value == null && len == 0)
             {
                 // do one try to see how much data we need
-                ret = Sysctl(name,  name_len, pBuffer, &bytesLength);
+                ret = Sysctl(name, name_len, value, &bytesLength);
                 if (ret != 0)
                 {
                     throw new InvalidOperationException(SR.Format(SR.InvalidSysctl, *name, Marshal.GetLastWin32Error()));
                 }
-                pBuffer = (byte*)Marshal.AllocHGlobal((int)bytesLength);
+                value = (byte*)Marshal.AllocHGlobal((int)bytesLength);
             }
-            ret = Sysctl(name,  name_len, pBuffer, &bytesLength);
+
+            ret = Sysctl(name, name_len, value, &bytesLength);
             if (ret != 0)
             {
-                if (value == null && len == 0)
-                {
-                    // This is case we allocated memory for caller
-                    Marshal.FreeHGlobal((IntPtr)pBuffer);
-                }
                 throw new InvalidOperationException(SR.Format(SR.InvalidSysctl, *name, Marshal.GetLastWin32Error()));
             }
 
-            value = pBuffer;
             len = (int)bytesLength;
-
-            return ret;
         }
     }
 }

diff --git a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs
index 90310c7..bc1920c 100644 (file)
--- a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs
+++ b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.GetProcInfo.cs
@@ -28,7 +28,7 @@ internal static partial class Interop
         private const int CTL_KERN = 1;
         private const int KERN_PROC = 14;
         private const int KERN_PROC_PROC = 8;
-        private const int KERN_PROC_PID  = 1;
+        private const int KERN_PROC_PID = 1;
         private const int KERN_PROC_INC_THREAD = 16;
 
         // From sys/_sigset.h
@@ -144,13 +144,13 @@ internal static partial class Interop
             private byte ki_rqindex;                    /* Run queue index */
             private byte ki_oncpu_old;                  /* Which cpu we are on (legacy) */
             private byte ki_lastcpu_old;                /* Last cpu we were on (legacy) */
-            public fixed byte ki_tdname[TDNAMLEN+1];    /* thread name */
-            private fixed byte ki_wmesg[WMESGLEN+1];    /* wchan message */
-            private fixed byte ki_login[LOGNAMELEN+1];  /* setlogin name */
-            private fixed byte ki_lockname[LOCKNAMELEN+1]; /* lock name */
-            public fixed byte ki_comm[COMMLEN+1];       /* command name */
-            private fixed byte ki_emul[KI_EMULNAMELEN+1]; /* emulation name */
-            private fixed byte ki_loginclass[LOGINCLASSLEN+1]; /* login class */
+            public fixed byte ki_tdname[TDNAMLEN + 1];    /* thread name */
+            private fixed byte ki_wmesg[WMESGLEN + 1];    /* wchan message */
+            private fixed byte ki_login[LOGNAMELEN + 1];  /* setlogin name */
+            private fixed byte ki_lockname[LOCKNAMELEN + 1]; /* lock name */
+            public fixed byte ki_comm[COMMLEN + 1];       /* command name */
+            private fixed byte ki_emul[KI_EMULNAMELEN + 1]; /* emulation name */
+            private fixed byte ki_loginclass[LOGINCLASSLEN + 1]; /* login class */
             private fixed byte ki_sparestrings[50];     /* spare string space */
             private fixed int ki_spareints[KI_NSPARE_INT]; /* spare room for growth */
             private int ki_oncpu;                       /* Which cpu we are on */
@@ -189,7 +189,6 @@ internal static partial class Interop
             int bytesLength = 0;
             byte* pBuffer = null;
             kinfo_proc* kinfo = null;
-            int ret;
 
             count = -1;
 
@@ -210,11 +209,7 @@ internal static partial class Interop
 
             try
             {
-                ret = Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength);
-                if (ret != 0 ) {
-                    throw new ArgumentOutOfRangeException(nameof(pid));
-                }
-
+                Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength);
                 kinfo = (kinfo_proc*)pBuffer;
                 if (kinfo->ki_structsize != sizeof(kinfo_proc))
                 {
@@ -224,10 +219,9 @@ internal static partial class Interop
 
                 count = (int)bytesLength / sizeof(kinfo_proc);
             }
-            catch
+            finally
             {
                 Marshal.FreeHGlobal((IntPtr)pBuffer);
-                throw;
             }
 
             return kinfo;

diff --git a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs
index 59dd664..031f56a 100644 (file)
--- a/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs
+++ b/src/libraries/Common/src/Interop/FreeBSD/Interop.Process.cs
@@ -50,7 +50,7 @@ internal static partial class Interop
                     if (list[i].ki_ppid == 0)
                     {
                         // skip kernel threads
-                        numProcesses-=1;
+                        numProcesses -= 1;
                     }
                     else
                     {
@@ -76,22 +76,19 @@ internal static partial class Interop
         /// <param name="pid">The PID of the process</param>
         public static unsafe string? GetProcPath(int pid)
         {
-            Span<int> sysctlName = stackalloc int[4];
+            Span<int> sysctlName = stackalloc int[] { CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, pid };
             byte* pBuffer = null;
             int bytesLength = 0;
 
-            sysctlName[0] = CTL_KERN;
-            sysctlName[1] = KERN_PROC;
-            sysctlName[2] = KERN_PROC_PATHNAME;
-            sysctlName[3] = pid;
-
-            if (Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength) != 0)
+            try
             {
-                return null;
+                Interop.Sys.Sysctl(sysctlName, ref pBuffer, ref bytesLength);
+                return System.Text.Encoding.UTF8.GetString(pBuffer, (int)bytesLength - 1);
+            }
+            finally
+            {
+                Marshal.FreeHGlobal((IntPtr)pBuffer);
             }
-
-            // TODO Fix freeing of pBuffer: https://github.com/dotnet/runtime/issues/43418
-            return System.Text.Encoding.UTF8.GetString(pBuffer, (int)bytesLength-1);
         }
 
         /// <summary>