Improve perf and correctness of UTF-8 marshaling

6 files changed, 245 insertions, 100 deletions

diff --git a/LibGit2Sharp/ContentChanges.cs b/LibGit2Sharp/ContentChanges.cs
index 3033bb86..207dc905 100644
--- a/LibGit2Sharp/ContentChanges.cs
+++ b/LibGit2Sharp/ContentChanges.cs
@@ -39,7 +39,7 @@ namespace LibGit2Sharp
 
         private int HunkCallback(GitDiffDelta delta, GitDiffRange range, IntPtr header, UIntPtr headerlen, IntPtr payload)
         {
-            string decodedContent = Utf8Marshaler.FromNative(header, (uint)headerlen);
+            string decodedContent = Utf8Marshaler.FromNative(header, (int)headerlen);
 
             AppendToPatch(decodedContent);
             return 0;
@@ -47,7 +47,7 @@ namespace LibGit2Sharp
 
         private int LineCallback(GitDiffDelta delta, GitDiffRange range, GitDiffLineOrigin lineorigin, IntPtr content, UIntPtr contentlen, IntPtr payload)
         {
-            string decodedContent = Utf8Marshaler.FromNative(content, (uint)contentlen);
+            string decodedContent = Utf8Marshaler.FromNative(content, (int)contentlen);
 
             string prefix;
 
diff --git a/LibGit2Sharp/Core/FilePathMarshaler.cs b/LibGit2Sharp/Core/FilePathMarshaler.cs
index 8a8f1ff1..d30132f1 100644
--- a/LibGit2Sharp/Core/FilePathMarshaler.cs
+++ b/LibGit2Sharp/Core/FilePathMarshaler.cs
@@ -3,43 +3,132 @@ using System.Runtime.InteropServices;
 
 namespace LibGit2Sharp.Core
 {
-    internal class FilePathMarshaler : Utf8Marshaler
+    /// <summary>
+    ///   This marshaler is to be used for capturing a UTF-8 string owned by libgit2 and
+    ///   converting it to a managed FilePath instance. The marshaler will not attempt to
+    ///   free the native pointer after conversion, because the memory is owned by libgit2.
+    ///
+    ///   Use this marshaler for return values, for example:
+    ///   [return: MarshalAs(UnmanagedType.CustomMarshaler,
+    ///                      MarshalTypeRef = typeof(FilePathNoCleanupMarshaler))]
+    /// </summary>
+    internal class FilePathNoCleanupMarshaler : FilePathMarshaler
+    {
+        private static readonly FilePathNoCleanupMarshaler staticInstance = new FilePathNoCleanupMarshaler();
+
+        public static new ICustomMarshaler GetInstance(String cookie)
+        {
+            return staticInstance;
+        }
+
+        #region ICustomMarshaler
+
+        public override void CleanUpNativeData(IntPtr pNativeData)
+        {
+        }
+
+        #endregion
+    }
+
+    /// <summary>
+    ///   This marshaler is to be used for sending managed FilePath instances to libgit2.
+    ///   The marshaler will allocate a buffer in native memory to hold the UTF-8 string
+    ///   and perform the encoding conversion using that buffer as the target. The pointer
+    ///   received by libgit2 will be to this buffer. After the function call completes, the
+    ///   native buffer is freed.
+    ///
+    ///   Use this marshaler for function parameters, for example:
+    ///   [DllImport(libgit2)]
+    ///   internal static extern int git_index_open(out IndexSafeHandle index,
+    ///       [MarshalAs(UnmanagedType.CustomMarshaler,
+    ///                  MarshalTypeRef = typeof(FilePathMarshaler))] FilePath indexpath);
+    /// </summary>
+    internal class FilePathMarshaler : ICustomMarshaler
     {
         private static readonly FilePathMarshaler staticInstance = new FilePathMarshaler();
 
-        public override IntPtr MarshalManagedToNative(object managedObj)
+        public static ICustomMarshaler GetInstance(String cookie)
+        {
+            return staticInstance;
+        }
+
+        #region ICustomMarshaler
+
+        public void CleanUpManagedData(Object managedObj)
+        {
+        }
+
+        public virtual void CleanUpNativeData(IntPtr pNativeData)
+        {
+            if (IntPtr.Zero != pNativeData)
+            {
+                Marshal.FreeHGlobal(pNativeData);
+            }
+        }
+
+        public int GetNativeDataSize()
+        {
+            // Not a value type
+            return -1;
+        }
+
+        public IntPtr MarshalManagedToNative(Object managedObj)
         {
-            if (managedObj == null)
+            if (null == managedObj)
             {
                 return IntPtr.Zero;
             }
 
-            if (!(managedObj is FilePath))
+            FilePath filePath = managedObj as FilePath;
+
+            if (null == filePath)
             {
                 throw new MarshalDirectiveException("FilePathMarshaler must be used on a FilePath.");
             }
 
-            return StringToNative(((FilePath)managedObj).Posix);
+            return FilePathMarshaler.FromManaged(filePath);
         }
 
-        public override object MarshalNativeToManaged(IntPtr pNativeData)
+        public Object MarshalNativeToManaged(IntPtr pNativeData)
         {
-            return (FilePath)NativeToString(pNativeData);
+            return FilePathMarshaler.FromNative(pNativeData);
         }
 
-        public static IntPtr FromManaged(FilePath managedObj)
+        #endregion
+
+        public static unsafe IntPtr FromManaged(FilePath filePath)
         {
-            return staticInstance.MarshalManagedToNative(managedObj);
+            if (null == filePath)
+            {
+                return IntPtr.Zero;
+            }
+
+            return Utf8Marshaler.FromManaged(filePath.Posix);
         }
 
-        public new static FilePath FromNative(IntPtr pNativeData)
+        public static unsafe FilePath FromNative(IntPtr pNativeData)
         {
-            return (FilePath)staticInstance.MarshalNativeToManaged(pNativeData);
+            if (IntPtr.Zero == pNativeData)
+            {
+                return null;
+            }
+
+            if (0 == Marshal.ReadByte(pNativeData))
+            {
+                return FilePath.Empty;
+            }
+
+            return (FilePath)Utf8Marshaler.FromNative(pNativeData);
         }
 
-        public new static ICustomMarshaler GetInstance(string cookie)
+        public static unsafe FilePath FromNative(IntPtr pNativeData, int length)
         {
-            return staticInstance;
+            if (0 == length)
+            {
+                return FilePath.Empty;
+            }
+
+            return (FilePath)Utf8Marshaler.FromNative(pNativeData, length);
         }
     }
 }
diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs
index 8f2cffd9..6ccaceff 100644
--- a/LibGit2Sharp/Core/NativeMethods.cs
+++ b/LibGit2Sharp/Core/NativeMethods.cs
@@ -190,11 +190,11 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 7)] [In] IntPtr[] parents);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_commit_message(GitObjectSafeHandle commit);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_commit_message_encoding(GitObjectSafeHandle commit);
 
         [DllImport(libgit2)]
@@ -459,7 +459,7 @@ namespace LibGit2Sharp.Core
         internal static extern void git_note_free(IntPtr note);
 
         [DllImport(libgit2)]
-        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_note_message(NoteSafeHandle note);
 
         [DllImport(libgit2)]
@@ -482,7 +482,7 @@ namespace LibGit2Sharp.Core
 
         [DllImport(libgit2)]
         internal static extern int git_note_default_ref(
-            [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] out string notes_ref,
+            [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))] out string notes_ref,
             RepositorySafeHandle repo);
 
         internal delegate int git_note_foreach_cb(
@@ -572,7 +572,7 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_reference_name(ReferenceSafeHandle reference);
 
         [DllImport(libgit2)]
@@ -596,7 +596,7 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string target);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_reference_symbolic_target(ReferenceSafeHandle reference);
 
         [DllImport(libgit2)]
@@ -612,7 +612,7 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_remote_name(RemoteSafeHandle remote);
 
         [DllImport(libgit2)]
@@ -623,7 +623,7 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string url);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_remote_url(RemoteSafeHandle remote);
 
         [DllImport(libgit2)]
@@ -708,7 +708,7 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(FilePathMarshaler))] FilePath path);
 
         [DllImport(libgit2)]
-        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(FilePathMarshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(FilePathNoCleanupMarshaler))]
         internal static extern FilePath git_repository_path(RepositorySafeHandle repository);
 
         [DllImport(libgit2)]
@@ -732,7 +732,7 @@ namespace LibGit2Sharp.Core
             RepositorySafeHandle repository);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(FilePathMarshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(FilePathNoCleanupMarshaler))]
         internal static extern FilePath git_repository_workdir(RepositorySafeHandle repository);
 
         [DllImport(libgit2)]
@@ -819,11 +819,11 @@ namespace LibGit2Sharp.Core
             [MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string tagName);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_tag_message(GitObjectSafeHandle tag);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_tag_name(GitObjectSafeHandle tag);
 
         [DllImport(libgit2)]
@@ -862,7 +862,7 @@ namespace LibGit2Sharp.Core
         internal static extern OidSafeHandle git_tree_entry_id(SafeHandle entry);
 
         [DllImport(libgit2)]
-        [return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
+        [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
         internal static extern string git_tree_entry_name(SafeHandle entry);
 
         [DllImport(libgit2)]
diff --git a/LibGit2Sharp/Core/Utf8Marshaler.cs b/LibGit2Sharp/Core/Utf8Marshaler.cs
index bbcd180b..9c192464 100644
--- a/LibGit2Sharp/Core/Utf8Marshaler.cs
+++ b/LibGit2Sharp/Core/Utf8Marshaler.cs
@@ -4,127 +4,183 @@ using System.Text;
 
 namespace LibGit2Sharp.Core
 {
+    /// <summary>
+    ///   This marshaler is to be used for capturing a UTF-8 string owned by libgit2 and
+    ///   converting it to a managed String instance. The marshaler will not attempt to
+    ///   free the native pointer after conversion, because the memory is owned by libgit2.
+    ///
+    ///   Use this marshaler for return values, for example:
+    ///   [return: MarshalAs(UnmanagedType.CustomMarshaler,
+    ///                      MarshalTypeRef = typeof(Utf8NoCleanupMarshaler))]
+    /// </summary>
+    internal class Utf8NoCleanupMarshaler : Utf8Marshaler
+    {
+        private static readonly Utf8NoCleanupMarshaler staticInstance = new Utf8NoCleanupMarshaler();
+
+        public new static ICustomMarshaler GetInstance(String cookie)
+        {
+            return staticInstance;
+        }
+
+        #region ICustomMarshaler
+
+        public override void CleanUpNativeData(IntPtr pNativeData)
+        {
+        }
+
+        #endregion
+    }
+
+    /// <summary>
+    ///   This marshaler is to be used for sending managed String instances to libgit2.
+    ///   The marshaler will allocate a buffer in native memory to hold the UTF-8 string
+    ///   and perform the encoding conversion using that buffer as the target. The pointer
+    ///   received by libgit2 will be to this buffer. After the function call completes, the
+    ///   native buffer is freed.
+    ///
+    ///   Use this marshaler for function parameters, for example:
+    ///   [DllImport(libgit2)]
+    ///   internal static extern int git_tag_delete(RepositorySafeHandle repo,
+    ///       [MarshalAs(UnmanagedType.CustomMarshaler,
+    ///                  MarshalTypeRef = typeof(Utf8Marshaler))] String tagName);
+    /// </summary>
     internal class Utf8Marshaler : ICustomMarshaler
     {
         private static readonly Utf8Marshaler staticInstance = new Utf8Marshaler();
-        private readonly bool ownsPointer;
 
-        internal Utf8Marshaler(bool ownsPointer = false)
+        public static ICustomMarshaler GetInstance(String cookie)
         {
-            this.ownsPointer = ownsPointer;
+            return staticInstance;
         }
 
-        #region ICustomMarshaler Members
+        #region ICustomMarshaler
 
-        public virtual IntPtr MarshalManagedToNative(object managedObj)
+        public void CleanUpManagedData(Object managedObj)
         {
-            if (managedObj == null)
-            {
-                return IntPtr.Zero;
-            }
+        }
 
-            if (!(managedObj is string))
+        public virtual void CleanUpNativeData(IntPtr pNativeData)
+        {
+            if (IntPtr.Zero != pNativeData)
             {
-                throw new MarshalDirectiveException("UTF8Marshaler must be used on a string.");
+                Marshal.FreeHGlobal(pNativeData);
             }
+        }
 
-            return StringToNative((string)managedObj);
+        public int GetNativeDataSize()
+        {
+            // Not a value type
+            return -1;
         }
 
-        protected static unsafe IntPtr StringToNative(string value)
+        public IntPtr MarshalManagedToNative(Object managedObj)
         {
-            // not null terminated
-            byte[] strbuf = Encoding.UTF8.GetBytes(value);
-            IntPtr buffer = Marshal.AllocHGlobal(strbuf.Length + 1);
-            Marshal.Copy(strbuf, 0, buffer, strbuf.Length);
+            if (null == managedObj)
+            {
+                return IntPtr.Zero;
+            }
 
-            // write the terminating null
-            var pBuf = (byte*)buffer;
-            pBuf[strbuf.Length] = 0;
+            String str = managedObj as String;
 
-            return buffer;
+            if (null == str)
+            {
+                throw new MarshalDirectiveException("Utf8Marshaler must be used on a string.");
+            }
+
+            return Utf8Marshaler.FromManaged(str);
         }
 
-        public virtual object MarshalNativeToManaged(IntPtr pNativeData)
+        public Object MarshalNativeToManaged(IntPtr pNativeData)
         {
-            return NativeToString(pNativeData);
+            return Utf8Marshaler.FromNative(pNativeData);
         }
 
-        protected static unsafe string NativeToString(IntPtr pNativeData)
+        #endregion
+
+        public static unsafe IntPtr FromManaged(String value)
         {
-            var walk = (byte*)pNativeData;
+            if (null == value)
+            {
+                return IntPtr.Zero;
+            }
 
-            // find the end of the string
-            while (*walk != 0)
+            int length = Encoding.UTF8.GetByteCount(value);
+            byte* buffer = (byte*)Marshal.AllocHGlobal(length + 1).ToPointer();
+
+            if (length > 0)
             {
-                walk++;
+                fixed (char* pValue = value)
+                {
+                    Encoding.UTF8.GetBytes(pValue, value.Length, buffer, length);
+                }
             }
 
-            var length = (uint)(walk - (byte*)pNativeData);
+            buffer[length] = 0;
 
-            return FromNative(pNativeData, length);
+            return new IntPtr(buffer);
         }
 
-        public static string FromNative(IntPtr pNativeData, uint length)
+        public static unsafe String FromNative(IntPtr pNativeData)
         {
-            // should not be null terminated
-            var strbuf = new byte[length];
+            if (IntPtr.Zero == pNativeData)
+            {
+                return null;
+            }
 
-            // skip the trailing null
-            Marshal.Copy(pNativeData, strbuf, 0, (int)length);
-            string data = Encoding.UTF8.GetString(strbuf);
-            return data;
-        }
+            byte* start = (byte*)pNativeData;
+            byte* walk = start;
 
-        public void CleanUpNativeData(IntPtr pNativeData)
-        {
-            if (ownsPointer)
-                Marshal.FreeHGlobal(pNativeData);
-        }
+            // Find the end of the string
+            while (*walk != 0)
+            {
+                walk++;
+            }
 
-        public void CleanUpManagedData(object managedObj)
-        {
+            if (walk == start)
+            {
+                return String.Empty;
+            }
+
+            return new String((sbyte*)pNativeData.ToPointer(), 0, (int)(walk - start), Encoding.UTF8);
         }
 
-        public int GetNativeDataSize()
+        public static unsafe String FromNative(IntPtr pNativeData, int length)
         {
-            return -1;
-        }
+            if (IntPtr.Zero == pNativeData)
+            {
+                return null;
+            }
 
-        #endregion
+            if (0 == length)
+            {
+                return String.Empty;
+            }
 
-        public static IntPtr FromManaged(string managedObj)
-        {
-            return staticInstance.MarshalManagedToNative(managedObj);
+            return new String((sbyte*)pNativeData.ToPointer(), 0, length, Encoding.UTF8);
         }
 
-        public static string FromNative(IntPtr pNativeData)
+        public static String Utf8FromBuffer(byte[] buffer)
         {
-            return (string)staticInstance.MarshalNativeToManaged(pNativeData);
-        }
+            if (null == buffer)
+            {
+                return null;
+            }
 
-        public static ICustomMarshaler GetInstance(string cookie)
-        {
-            return staticInstance;
-        }
+            int length = 0;
+            int stop = buffer.Length;
 
-        public static string Utf8FromBuffer(byte[] buffer)
-        {
-            int nullTerminator;
-            for (nullTerminator = 0; nullTerminator < buffer.Length; nullTerminator++)
+            while (length < stop &&
+                   0 != buffer[length])
             {
-                if (buffer[nullTerminator] == 0)
-                {
-                    break;
-                }
+                length++;
             }
 
-            if (nullTerminator == 0)
+            if (0 == length)
             {
-                return null;
+                return String.Empty;
             }
 
-            return Encoding.UTF8.GetString(buffer, 0, nullTerminator);
+            return Encoding.UTF8.GetString(buffer, 0, length);
         }
     }
 }
diff --git a/LibGit2Sharp/RemoteCallbacks.cs b/LibGit2Sharp/RemoteCallbacks.cs
index 8afb2316..c5bc8e0e 100644
--- a/LibGit2Sharp/RemoteCallbacks.cs
+++ b/LibGit2Sharp/RemoteCallbacks.cs
@@ -76,7 +76,7 @@ namespace LibGit2Sharp
 
             if (onProgress != null)
             {
-                string message = Utf8Marshaler.FromNative(str, (uint)len);
+                string message = Utf8Marshaler.FromNative(str, len);
                 onProgress(message);
             }
         }
diff --git a/LibGit2Sharp/TreeChanges.cs b/LibGit2Sharp/TreeChanges.cs
index 1946364a..876a7b4c 100644
--- a/LibGit2Sharp/TreeChanges.cs
+++ b/LibGit2Sharp/TreeChanges.cs
@@ -51,7 +51,7 @@ namespace LibGit2Sharp
 
         private int PrintCallBack(GitDiffDelta delta, GitDiffRange range, GitDiffLineOrigin lineorigin, IntPtr content, UIntPtr contentlen, IntPtr payload)
         {
-            string formattedoutput = Utf8Marshaler.FromNative(content, (uint)contentlen);
+            string formattedoutput = Utf8Marshaler.FromNative(content, (int)contentlen);
 
             TreeEntryChanges currentChange = AddFileChange(delta, lineorigin);
             AddLineChange(currentChange, lineorigin);