8000 Fix up marshalling of strings returned by libgit2 · rlazev/libgit2sharp@69229e6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 69229e6

Browse files
tclemnulltoken
tclem
authored and
nulltoken
committed
Fix up marshalling of strings returned by libgit2
Previously we were doing this which is not good: return Marshal.PtrToStringAnsi(intPtr); You can see the failing test that was added to ConfigurationFixture.cs to demonstrate the problem (setting and getting a user name with unicode chars in the config). This brought up the large problem of how we were dealing with libgit2 methods that returned string values. There was another subtle issue in the Utf8Marshaler where we were freeing memory that we didn't own.
1 parent 4f23c34 commit 69229e6

13 files changed

+79
-44
lines changed

‎LibGit2Sharp.Tests/ConfigurationFixture.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,28 @@ public void CanSetStringValue()
231231
}
232232
}
233233

234+
[Fact]
235+
public void CanSetAndReadUnicodeStringValue()
236+
{
237+
var path = BuildTemporaryCloneOfTestRepo(StandardTestRepoPath);
23 A3E2 8+
using (var repo = new Repository(path.RepositoryPath))
239+
{
240+
repo.Config.Set("unittests.stringsetting", "Juliën");
241+
242+
AssertValueInLocalConfigFile(path.RepositoryPath, "stringsetting = Juliën$");
243+
244+
string val = repo.Config.Get("unittests.stringsetting", "");
245+
val.ShouldEqual("Juliën");
246+
}
247+
248+
// Make sure the change is permanent
249+
using (var repo = new Repository(path.RepositoryPath))
250+
{
251+
string val = repo.Config.Get("unittests.stringsetting", "");
252+
val.ShouldEqual("Juliën");
253+
}
254+
}
255+
234256
[Fact]
235257
public void ReadingUnsupportedTypeThrows()
236258
{

‎LibGit2Sharp/Commit.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ internal static Commit BuildFromPtr(IntPtr obj, ObjectId id, Repository repo)
124124

125125
return new Commit(id, treeId, repo)
126126
{
127-
Message = NativeMethods.git_commit_message(obj).MarshallAsString(), //TODO: Turn into string.Empty if null
127+
Message = NativeMethods.git_commit_message(obj),
128128
Encoding = RetrieveEncodingOf(obj),
129129
Author = new Signature(NativeMethods.git_commit_author(obj)),
130130
Committer = new Signature(NativeMethods.git_commit_committer(obj)),
@@ -133,7 +133,7 @@ internal static Commit BuildFromPtr(IntPtr obj, ObjectId id, Repository repo)
133133

134134
private static string RetrieveEncodingOf(IntPtr obj)
135135
{
136-
string encoding = NativeMethods.git_commit_message_encoding(obj).MarshallAsString();
136+
string encoding = NativeMethods.git_commit_message_encoding(obj);
137137

138138
return encoding ?? "UTF-8";
139139
}

‎LibGit2Sharp/Configuration.cs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ protected virtual void Dispose(bool disposing)
115115
systemHandle.SafeDispose();
116116
}
117117

118-
private static T ProcessReadResult<T>(int res, T value, T defaultValue, Func<object, T> postProcessor = null)
118+
private static T ProcessReadResult<T>(int res, T value, T defaultValue)
119119
{
120120
if (res == (int)GitErrorCode.GIT_ENOTFOUND)
121121
{
@@ -124,12 +124,7 @@ private static T ProcessReadResult<T>(int res, T value, T defaultValue, Func<obj
124124

125125
Ensure.Success(res);
126126

127-
if (postProcessor == null)
128-
{
129-
return value;
130-
}
131-
132-
return postProcessor(value);
127+
return value;
133128
}
134129

135130
private readonly IDictionary<Type, Func<string, object, ConfigurationSafeHandle, object>> configurationTypedRetriever = ConfigurationTypedRetriever();
@@ -161,9 +156,9 @@ private static Dictionary<Type, Func<string, object, ConfigurationSafeHandle, ob
161156

162157
dic.Add(typeof(string), (key, dv, handle) =>
163158
{
164-
IntPtr value;
159+
string value;
165160
int res = NativeMethods.git_config_get_string(handle, key, out value);
166-
return ProcessReadResult<object>(res, value, dv, s => ((IntPtr)s).MarshallAsString());
161+
return ProcessReadResult<object>(res, value, dv);
167162
});
168163

169164
return dic;

‎LibGit2Sharp/Core/Ensure.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public static void Success(int result, bool allowPositiveResult = false)
6060
return;
6161
}
6262

63-
string errorMessage = NativeMethods.git_lasterror().MarshallAsString();
63+
string errorMessage = NativeMethods.git_lasterror();
6464

6565
throw new LibGit2Exception(
6666
String.Format(CultureInfo.InvariantCulture, "An error was raised by libgit2. Error code = {0} ({1}).{2}{3}", Enum.GetName(typeof(GitErrorCode), result), result, Environment.NewLine, errorMessage));

‎LibGit2Sharp/Core/IntPtrExtensions.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,6 @@ namespace LibGit2Sharp.Core
55
{
66
internal static class IntPtrExtensions
77
{
8-
public static string MarshallAsString(this IntPtr intPtr)
9-
{
10-
return Marshal.PtrToStringAnsi(intPtr);
11-
}
12-
138
public static GitOid MarshalAsOid(this IntPtr intPtr)
149
{
1510
return (GitOid)Marshal.PtrToStructure(intPtr, typeof(GitOid));

‎LibGit2Sharp/Core/NativeMethods.cs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,12 @@ public static extern int git_commit_create(
7777
[MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 7)] [In] IntPtr[] parents);
7878

7979
[DllImport(libgit2)]
80-
public static extern IntPtr git_commit_message(IntPtr commit);
80+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
81+
public static extern string git_commit_message(IntPtr commit);
8182

8283
[DllImport(libgit2)]
83-
public static extern IntPtr git_commit_message_encoding(IntPtr commit);
84+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
85+
public static extern string git_commit_message_encoding(IntPtr commit);
8486

8587
[DllImport(libgit2)]
8688
public static extern int git_commit_parent(out IntPtr parentCommit, IntPtr commit, uint n);
@@ -129,7 +131,7 @@ public static extern int git_config_get_int64(
129131
public static extern int git_config_get_string(
130132
ConfigurationSafeHandle cfg,
131133
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name,
132-
out IntPtr value);
134+
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] out string value);
133135

134136
[DllImport(libgit2)]
135137
public static extern int git_config_open_global(out ConfigurationSafeHandle cfg);
@@ -198,7 +200,8 @@ public static extern int git_index_find(
198200
public static extern int git_index_write(IndexSafeHandle index);
199201

200202
[DllImport(libgit2)]
201-
public static extern IntPtr git_lasterror();
203+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
204+
public static extern string git_lasterror();
202205

203206
[DllImport(libgit2)]
204207
public static extern void git_object_free(IntPtr obj);
@@ -247,7 +250,8 @@ public static extern int git_reference_lookup(
247250
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name);
248251

249252
[DllImport(libgit2)]
250-
public static extern IntPtr git_reference_name(IntPtr reference);
253+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
254+
public static extern string git_reference_name(IntPtr reference);
251255

252256
[DllImport(libgit2)]
253257
public static extern IntPtr git_reference_oid(IntPtr reference);
@@ -270,7 +274,8 @@ public static extern int git_reference_set_target(
270274
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string target);
271275

272276
[DllImport(libgit2)]
273-
public static extern IntPtr git_reference_target(IntPtr reference);
277+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
278+
public static extern string git_reference_target(IntPtr reference);
274279

275280
[DllImport(libgit2)]
276281
public static extern GitReferenceType git_reference_type(IntPtr reference);
@@ -285,7 +290,8 @@ public static extern int git_remote_load(
285290
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name);
286291

287292
[DllImport(libgit2)]
288-
public static extern IntPtr git_remote_name(RemoteSafeHandle remote);
293+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
294+
public static extern string git_remote_name(RemoteSafeHandle remote);
289295

290296
[DllImport(libgit2)]
291297
public static extern int git_remote_new(
@@ -295,7 +301,8 @@ public static extern int git_remote_new(
295301
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string name);
296302

297303
[DllImport(libgit2)]
298-
public static extern IntPtr git_remote_url(RemoteSafeHandle remote);
304+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
305+
public static extern string git_remote_url(RemoteSafeHandle remote);
299306

300307
[DllImport(libgit2)]
301308
public static extern int git_remote_save(RemoteSafeHandle remote);
@@ -340,10 +347,12 @@ public static extern int git_repository_open(
340347
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string path);
341348

342349
[DllImport(libgit2)]
343-
public static extern IntPtr git_repository_path(RepositorySafeHandle repository);
350+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
351+
public static extern string git_repository_path(RepositorySafeHandle repository);
344352

345353
[DllImport(libgit2)]
346-
public static extern IntPtr git_repository_workdir(RepositorySafeHandle repository);
354+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
355+
public static extern string git_repository_workdir(RepositorySafeHandle repository);
347356

348357
[DllImport(libgit2)]
349358
public static extern void git_revwalk_free(IntPtr walker);
@@ -417,10 +426,12 @@ public static extern int git_tag_delete(
417426
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))] string tagName);
418427

419428
[DllImport(libgit2)]
420-
public static extern IntPtr git_tag_message(IntPtr tag);
429+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
430+
public static extern string git_tag_message(IntPtr tag);
421431

422432
[DllImport(libgit2)]
423-
public static extern IntPtr git_tag_name(IntPtr tag);
433+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
434+
public static extern string git_tag_name(IntPtr tag);
424435

425436
[DllImport(libgit2)]
426437
public static extern IntPtr git_tag_tagger(IntPtr tag);
@@ -455,7 +466,8 @@ public static extern IntPtr git_tree_entry_byname(
455466
public static extern IntPtr git_tree_entry_id(IntPtr entry);
456467

457468
[DllImport(libgit2)]
458-
public static extern IntPtr git_tree_entry_name(IntPtr entry);
469+
[return : MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef = typeof(Utf8Marshaler))]
470+
public static extern string git_tree_entry_name(IntPtr entry);
459471

460472
[DllImport(libgit2)]
461473
public static extern GitObjectType git_tree_entry_type(IntPtr entry);

‎LibGit2Sharp/Core/Utf8Marshaler.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,15 @@ namespace LibGit2Sharp.Core
66
{
77
internal class Utf8Marshaler : ICustomMarshaler
88
{
9-
private static Utf8Marshaler staticInstance;
9+
static Utf8Marshaler staticInstance;
10+
readonly bool ownsPointer;
11+
12+
internal Utf8Marshaler(bool ownsPointer = false)
13+
{
14+
this.ownsPointer = ownsPointer;
15+
}
16+
17+
#region ICustomMarshaler Members
1018

1119
public unsafe IntPtr MarshalManagedToNative(object managedObj)
1220
{
@@ -54,7 +62,8 @@ public unsafe object MarshalNativeToManaged(IntPtr pNativeData)
5462

5563
public void CleanUpNativeData(IntPtr pNativeData)
5664
{
57-
Marshal.FreeHGlobal(pNativeData);
65+
if (ownsPointer)
66+
Marshal.FreeHGlobal(pNativeData);
5867
}
5968

6069
public void CleanUpManagedData(object managedObj)
@@ -66,6 +75,8 @@ public int GetNativeDataSize()
6675
return -1;
6776
}
6877

78+
#endregion
79+
6980
public static ICustomMarshaler GetInstance(string cookie)
7081
{
7182
if (staticInstance == null)

‎LibGit2Sharp/Reference.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ private static T BuildFromPtr<T>(IntPtr ptr, Repository repo) where T : Referenc
3232
return default(T);
3333
}
3434

35-
string name = NativeMethods.git_reference_name(ptr).MarshallAsString();
35+
string name = NativeMethods.git_reference_name(ptr);
3636
GitReferenceType type = NativeMethods.git_reference_type(ptr);
3737

3838
Reference reference;
@@ -41,7 +41,7 @@ private static T BuildFromPtr<T>(IntPtr ptr, Repository repo) where T : Referenc
4141
{
4242
case GitReferenceType.Symbolic:
4343
IntPtr resolveRef;
44-
var targetIdentifier = NativeMethods.git_reference_target(ptr).MarshallAsString();
44+
var targetIdentifier = NativeMethods.git_reference_target(ptr);
4545
int res = NativeMethods.git_reference_resolve(out resolveRef, ptr);
4646

4747
if (res == (int)GitErrorCode.GIT_ENOTFOUND)

‎LibGit2Sharp/Remote.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ internal static Remote CreateFromPtr(RemoteSafeHandle handle)
1818
return null;
1919
}
2020

21-
IntPtr namePtr = NativeMethods.git_remote_name(handle);
22-
IntPtr urlPtr = NativeMethods.git_remote_url(handle);
21+
string name = NativeMethods.git_remote_name(handle);
22+
string url = NativeMethods.git_remote_url(handle);
2323

2424
var remote = new Remote
2525
{
26-
Name = namePtr.MarshallAsString(),
27-
Url = urlPtr.MarshallAsString(),
26+
Name = name,
27+
Url = url,
2828
};
2929

3030
return remote;

‎LibGit2Sharp/Repository.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ public static Repository Init(string path, bool isBare = false)
179179
int res = NativeMethods.git_repository_init(out repo, PosixPathHelper.ToPosix(path), isBare);
180180
Ensure.Success(res);
181181

182-
string normalizedPath = NativeMethods.git_repository_path(repo).MarshallAsString();
182+
string normalizedPath = NativeMethods.git_repository_path(repo);
183183
repo.SafeDispose();
184184

185185
string nativePath = PosixPathHelper.ToNative(normalizedPath);

‎LibGit2Sharp/RepositoryInformation.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ internal RepositoryInformation(Repository repo, bool isBare)
1515
this.repo = repo;
1616
IsBare = isBare;
1717

18-
string posixPath = NativeMethods.git_repository_path(repo.Handle).MarshallAsString();
19-
string posixWorkingDirectoryPath = NativeMethods.git_repository_workdir(repo.Handle).MarshallAsString();
18+
string posixPath = NativeMethods.git_repository_path(repo.Handle);
19+
string posixWorkingDirectoryPath = NativeMethods.git_repository_workdir(repo.Handle);
2020

2121
Path = PosixPathHelper.ToNative(posixPath);
2222
WorkingDirectory = PosixPathHelper.ToNative(posixWorkingDirectoryPath);

‎LibGit2Sharp/TagAnnotation.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ internal static TagAnnotation BuildFromPtr(IntPtr obj, ObjectId id, Repository r
4646

4747
return new TagAnnotation(id)
4848
{
49-
Message = NativeMethods.git_tag_message(obj).MarshallAsString(),
50-
Name = NativeMethods.git_tag_name(obj).MarshallAsString(),
49+
Message = NativeMethods.git_tag_message(obj),
50+
Name = NativeMethods.git_tag_name(obj),
5151
Tagger = new Signature(NativeMethods.git_tag_tagger(obj)),
5252
targetBuilder = new Lazy<GitObject>(() => repo.Lookup<GitObject>(targetOid))
5353
};

‎LibGit2Sharp/TreeEntry.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ internal TreeEntry(IntPtr obj, ObjectId parentTreeId, Repository repo)
2828
target = new Lazy<GitObject>(RetreiveTreeEntryTarget);
2929

3030
Attributes = (int)NativeMethods.git_tree_entry_attributes(obj);
31-
Name = NativeMethods.git_tree_entry_name(obj).MarshallAsString();
31+
Name = NativeMethods.git_tree_entry_name(obj);
3232
}
3333

3434
/// <summary>

0 commit comments

Comments
 (0)
0