From dfc5de4c63d1251fe2cb005921538b24ec562774 Mon Sep 17 00:00:00 2001 From: Tim Clem Date: Fri, 15 Jun 2012 14:24:50 -0700 Subject: [PATCH 1/5] Cleanup error marshalling, generating code --- LibGit2Sharp/Core/Ensure.cs | 27 ++++------ LibGit2Sharp/Core/GitError.cs | 2 +- .../{GitErrorType.cs => GitErrorCategory.cs} | 2 +- LibGit2Sharp/Core/GitErrorCode.cs | 2 +- LibGit2Sharp/Core/GitErrorMarshaler.cs | 44 ++++++++++++++++ .../Core/Handles/GitErrorSafeHandle.cs | 12 ----- LibGit2Sharp/Core/NativeMethods.cs | 3 +- LibGit2Sharp/LibGit2Sharp.csproj | 4 +- LibGit2Sharp/LibGit2SharpException.cs | 50 ++++++++++--------- 9 files changed, 87 insertions(+), 59 deletions(-) rename LibGit2Sharp/Core/{GitErrorType.cs => GitErrorCategory.cs} (90%) create mode 100644 LibGit2Sharp/Core/GitErrorMarshaler.cs delete mode 100644 LibGit2Sharp/Core/Handles/GitErrorSafeHandle.cs diff --git a/LibGit2Sharp/Core/Ensure.cs b/LibGit2Sharp/Core/Ensure.cs index fe5266bb6..55ecf7fe8 100644 --- a/LibGit2Sharp/Core/Ensure.cs +++ b/LibGit2Sharp/Core/Ensure.cs @@ -62,26 +62,19 @@ public static void Success(int result, bool allowPositiveResult = false) return; } - string errorMessage; - GitError error = NativeMethods.giterr_last().MarshalAsGitError(); - - + var error = NativeMethods.giterr_last(); if (error == null) { - error = new GitError { Klass = GitErrorType.Unknown, Message = IntPtr.Zero }; - errorMessage = "No error message has been provided by the native library"; - } - else - { - errorMessage = Utf8Marshaler.FromNative(error.Message); + throw new LibGit2SharpException( + (GitErrorCode)result, + GitErrorCategory.Unknown, + "No error message has been provided by the native library"); } throw new LibGit2SharpException( - String.Format(CultureInfo.InvariantCulture, "An error was raised by libgit2. Class = {0} ({1}).{2}{3}", - error.Klass, - result, - Environment.NewLine, - errorMessage)); + (GitErrorCode)result, + error.Category, + Utf8Marshaler.FromNative(error.Message)); } /// @@ -108,8 +101,8 @@ public static void GitObjectIsNotNull(GitObject gitObject, string identifier) } throw new LibGit2SharpException(string.Format(CultureInfo.InvariantCulture, - "No valid git object identified by '{0}' exists in the repository.", - identifier)); + "No valid git object identified by '{0}' exists in the repository.", + identifier)); } } } diff --git a/LibGit2Sharp/Core/GitError.cs b/LibGit2Sharp/Core/GitError.cs index 3c75b0589..0041097da 100644 --- a/LibGit2Sharp/Core/GitError.cs +++ b/LibGit2Sharp/Core/GitError.cs @@ -7,6 +7,6 @@ namespace LibGit2Sharp.Core internal class GitError { public IntPtr Message; - public GitErrorType Klass; + public GitErrorCategory Category; } } diff --git a/LibGit2Sharp/Core/GitErrorType.cs b/LibGit2Sharp/Core/GitErrorCategory.cs similarity index 90% rename from LibGit2Sharp/Core/GitErrorType.cs rename to LibGit2Sharp/Core/GitErrorCategory.cs index 96835ccb0..afda14891 100644 --- a/LibGit2Sharp/Core/GitErrorType.cs +++ b/LibGit2Sharp/Core/GitErrorCategory.cs @@ -1,6 +1,6 @@ namespace LibGit2Sharp.Core { - internal enum GitErrorType + public enum GitErrorCategory { Unknown = -1, NoMemory, diff --git a/LibGit2Sharp/Core/GitErrorCode.cs b/LibGit2Sharp/Core/GitErrorCode.cs index d536c0328..5da44fb74 100644 --- a/LibGit2Sharp/Core/GitErrorCode.cs +++ b/LibGit2Sharp/Core/GitErrorCode.cs @@ -1,6 +1,6 @@ namespace LibGit2Sharp.Core { - internal enum GitErrorCode + public enum GitErrorCode { Ok = 0, Error = -1, diff --git a/LibGit2Sharp/Core/GitErrorMarshaler.cs b/LibGit2Sharp/Core/GitErrorMarshaler.cs new file mode 100644 index 000000000..df121691a --- /dev/null +++ b/LibGit2Sharp/Core/GitErrorMarshaler.cs @@ -0,0 +1,44 @@ +using System; +using System.Runtime.InteropServices; + +namespace LibGit2Sharp.Core +{ + internal class GitErrorMarshaler : ICustomMarshaler + { + static readonly GitErrorMarshaler staticInstance = new GitErrorMarshaler(); + + public void CleanUpManagedData(object managedObj) + { + } + + public void CleanUpNativeData(IntPtr pNativeData) + { + Marshal.FreeHGlobal(pNativeData); + } + + public int GetNativeDataSize() + { + return -1; + } + + public IntPtr MarshalManagedToNative(object managedObj) + { + throw new NotImplementedException(); + } + + public object MarshalNativeToManaged(IntPtr pNativeData) + { + return NativeToGitError(pNativeData); + } + + protected GitError NativeToGitError(IntPtr pNativeData) + { + return (GitError)Marshal.PtrToStructure(pNativeData, typeof(GitError)); + } + + public static ICustomMarshaler GetInstance(string cookie) + { + return staticInstance; + } + } +} diff --git a/LibGit2Sharp/Core/Handles/GitErrorSafeHandle.cs b/LibGit2Sharp/Core/Handles/GitErrorSafeHandle.cs deleted file mode 100644 index 9a222a482..000000000 --- a/LibGit2Sharp/Core/Handles/GitErrorSafeHandle.cs +++ /dev/null @@ -1,12 +0,0 @@ -using System.Runtime.InteropServices; - -namespace LibGit2Sharp.Core.Handles -{ - internal class GitErrorSafeHandle : NotOwnedSafeHandleBase - { - public GitError MarshalAsGitError() - { - return (GitError)Marshal.PtrToStructure(handle, typeof(GitError)); - } - } -} diff --git a/LibGit2Sharp/Core/NativeMethods.cs b/LibGit2Sharp/Core/NativeMethods.cs index 049e03b15..6dae14493 100644 --- a/LibGit2Sharp/Core/NativeMethods.cs +++ b/LibGit2Sharp/Core/NativeMethods.cs @@ -65,7 +65,8 @@ public static bool RepositoryStateChecker(RepositorySafeHandle repositoryPtr, Fu } [DllImport(libgit2)] - public static extern GitErrorSafeHandle giterr_last(); + [return: MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(GitErrorMarshaler))] + public static extern GitError giterr_last(); [DllImport(libgit2)] public static extern int git_blob_create_fromdisk( diff --git a/LibGit2Sharp/LibGit2Sharp.csproj b/LibGit2Sharp/LibGit2Sharp.csproj index 60733088c..b4cf77f85 100644 --- a/LibGit2Sharp/LibGit2Sharp.csproj +++ b/LibGit2Sharp/LibGit2Sharp.csproj @@ -77,10 +77,10 @@ - + + - diff --git a/LibGit2Sharp/LibGit2SharpException.cs b/LibGit2Sharp/LibGit2SharpException.cs index 4062ba1c1..c6d811af8 100644 --- a/LibGit2Sharp/LibGit2SharpException.cs +++ b/LibGit2Sharp/LibGit2SharpException.cs @@ -1,50 +1,41 @@ using System; +using System.Globalization; +using LibGit2Sharp.Core; namespace LibGit2Sharp { /// /// The exception that is thrown when an error occurs during application execution. /// - [Obsolete("This type will be removed in the next release. Please use LibGit2SharpException instead.")] - public class LibGit2Exception : LibGit2SharpException + public class LibGit2SharpException : Exception { + readonly bool isLibraryError; + /// - /// Initializes a new instance of the class. + /// The error code originally returned by libgit2. /// - public LibGit2Exception() - { - } + public GitErrorCode Code { get; private set; } /// - /// Initializes a new instance of the class with a specified error message. + /// The category of error raised by libgit2. /// - /// A message that describes the error. - public LibGit2Exception(string message) - : base(message) - { - } + public GitErrorCategory Category { get; private set; } /// - /// Initializes a new instance of the class with a specified error message and a reference to the inner exception that is the cause of this exception. + /// Initializes a new instance of the class. /// - /// The error message that explains the reason for the exception. - /// The exception that is the cause of the current exception. If the parameter is not a null reference, the current exception is raised in a catch block that handles the inner exception. - public LibGit2Exception(string message, Exception innerException) - : base(message, innerException) + public LibGit2SharpException() { } - } - /// - /// The exception that is thrown when an error occurs during application execution. - /// - public class LibGit2SharpException : Exception - { /// /// Initializes a new instance of the class. /// - public LibGit2SharpException() + public LibGit2SharpException(GitErrorCode code, GitErrorCategory category, string message) : base(message) { + Code = code; + Category = category; + isLibraryError = true; } /// @@ -65,5 +56,16 @@ public LibGit2SharpException(string message, Exception innerException) : base(message, innerException) { } + + public override string ToString() + { + return isLibraryError + ? String.Format(CultureInfo.InvariantCulture, "An error was raised by libgit2. Class = {0} ({1}).{2}{3}", + Category, + Code, + Environment.NewLine, + Message) + : base.ToString(); + } } } From 5f61f26736683239f89aeba848b76925ddc72e70 Mon Sep 17 00:00:00 2001 From: Tim Clem Date: Mon, 18 Jun 2012 08:45:45 -0700 Subject: [PATCH 2/5] Add libgit2exception back in --- LibGit2Sharp/LibGit2SharpException.cs | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/LibGit2Sharp/LibGit2SharpException.cs b/LibGit2Sharp/LibGit2SharpException.cs index c6d811af8..c975cf635 100644 --- a/LibGit2Sharp/LibGit2SharpException.cs +++ b/LibGit2Sharp/LibGit2SharpException.cs @@ -4,6 +4,39 @@ namespace LibGit2Sharp { + /// + /// The exception that is thrown when an error occurs during application execution. + /// + [Obsolete("This type will be removed in the next release. Please use LibGit2SharpException instead.")] + public class LibGit2Exception : LibGit2SharpException + { + /// + /// Initializes a new instance of the class. + /// + public LibGit2Exception() + { + } + + /// + /// Initializes a new instance of the class with a specified error message. + /// + /// A message that describes the error. + public LibGit2Exception(string message) + : base(message) + { + } + + /// + /// Initializes a new instance of the class with a specified error message and a reference to the inner exception that is the cause of this exception. + /// + /// The error message that explains the reason for the exception. + /// The exception that is the cause of the current exception. If the parameter is not a null reference, the current exception is raised in a catch block that handles the inner exception. + public LibGit2Exception(string message, Exception innerException) + : base(message, innerException) + { + } + } + /// /// The exception that is thrown when an error occurs during application execution. /// From 08374d78c3f68cfaa6bcfd348e74ae8a15b7cd3b Mon Sep 17 00:00:00 2001 From: Tim Clem Date: Mon, 18 Jun 2012 11:58:52 -0700 Subject: [PATCH 3/5] Don't exposes libgit2 error code/class directly --- LibGit2Sharp/LibGit2SharpException.cs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/LibGit2Sharp/LibGit2SharpException.cs b/LibGit2Sharp/LibGit2SharpException.cs index c975cf635..7ce9445e4 100644 --- a/LibGit2Sharp/LibGit2SharpException.cs +++ b/LibGit2Sharp/LibGit2SharpException.cs @@ -42,18 +42,10 @@ public LibGit2Exception(string message, Exception innerException) /// public class LibGit2SharpException : Exception { + readonly GitErrorCode code; + readonly GitErrorCategory category; readonly bool isLibraryError; - /// - /// The error code originally returned by libgit2. - /// - public GitErrorCode Code { get; private set; } - - /// - /// The category of error raised by libgit2. - /// - public GitErrorCategory Category { get; private set; } - /// /// Initializes a new instance of the class. /// @@ -66,8 +58,8 @@ public LibGit2SharpException() /// public LibGit2SharpException(GitErrorCode code, GitErrorCategory category, string message) : base(message) { - Code = code; - Category = category; + Data["libgit2.code"] = this.code = code; + Data["libgit2.class"] = this.category = category; isLibraryError = true; } @@ -94,8 +86,8 @@ public override string ToString() { return isLibraryError ? String.Format(CultureInfo.InvariantCulture, "An error was raised by libgit2. Class = {0} ({1}).{2}{3}", - Category, - Code, + category, + code, Environment.NewLine, Message) : base.ToString(); From 5e28e019310110405174c33f493325befb3d1167 Mon Sep 17 00:00:00 2001 From: Tim Clem Date: Thu, 21 Jun 2012 10:47:07 -0700 Subject: [PATCH 4/5] Expose ErrorCode and ErrorCategory as readonly --- LibGit2Sharp/LibGit2SharpException.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/LibGit2Sharp/LibGit2SharpException.cs b/LibGit2Sharp/LibGit2SharpException.cs index 7ce9445e4..43bf26f11 100644 --- a/LibGit2Sharp/LibGit2SharpException.cs +++ b/LibGit2Sharp/LibGit2SharpException.cs @@ -82,6 +82,22 @@ public LibGit2SharpException(string message, Exception innerException) { } + /// + /// The specific libgit2 error code. + /// + public GitErrorCode Code + { + get { return Data.Contains("libgit2.code") ? (GitErrorCode)Data["libgit2.code"] : GitErrorCode.Error; } + } + + /// + /// The specific libgit2 error class. + /// + public GitErrorCategory Category + { + get { return Data.Contains("libgit2.class") ? (GitErrorCategory)Data["libgit2.class"] : GitErrorCategory.Unknown; } + } + public override string ToString() { return isLibraryError From a820e0ef17902c463bc5cf84038bb371844b9b3b Mon Sep 17 00:00:00 2001 From: Tim Clem Date: Thu, 21 Jun 2012 11:14:24 -0700 Subject: [PATCH 5/5] Different strategy for handling null errors --- LibGit2Sharp/Core/GitErrorMarshaler.cs | 5 ++++- LibGit2Sharp/LibGit2SharpException.cs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/LibGit2Sharp/Core/GitErrorMarshaler.cs b/LibGit2Sharp/Core/GitErrorMarshaler.cs index df121691a..b3ba7a425 100644 --- a/LibGit2Sharp/Core/GitErrorMarshaler.cs +++ b/LibGit2Sharp/Core/GitErrorMarshaler.cs @@ -13,7 +13,10 @@ public void CleanUpManagedData(object managedObj) public void CleanUpNativeData(IntPtr pNativeData) { - Marshal.FreeHGlobal(pNativeData); + if (pNativeData != IntPtr.Zero) + { + Marshal.FreeHGlobal(pNativeData); + } } public int GetNativeDataSize() diff --git a/LibGit2Sharp/LibGit2SharpException.cs b/LibGit2Sharp/LibGit2SharpException.cs index 43bf26f11..92d455884 100644 --- a/LibGit2Sharp/LibGit2SharpException.cs +++ b/LibGit2Sharp/LibGit2SharpException.cs @@ -58,7 +58,7 @@ public LibGit2SharpException() /// public LibGit2SharpException(GitErrorCode code, GitErrorCategory category, string message) : base(message) { - Data["libgit2.code"] = this.code = code; + Data["libgit2.code"] = this.code = code; Data["libgit2.class"] = this.category = category; isLibraryError = true; }