Skip to content
This repository was archived by the owner on Feb 3, 2023. It is now read-only.

Commit 1f4f5fc

Browse files
author
J Wyman
committed
Fixes based on PR feedback
1 parent d2c4b91 commit 1f4f5fc

File tree

8 files changed

+137
-135
lines changed

8 files changed

+137
-135
lines changed

‎LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public void SmugdeIsNotCalledForFileWhichDoesNotMatchAnAttributeEntry()
1515
const string decodedInput = "This is a substitution cipher";
1616
const string encodedInput = "Guvf vf n fhofgvghgvba pvcure";
1717

18-
var attributes = new List<FilterAttributeEntry> { new FilterAttributeEntry("filter=rot13") };
18+
var attributes = new List<FilterAttributeEntry> { new FilterAttributeEntry("rot13") };
1919
var filter = new SubstitutionCipherFilter("cipher-filter", attributes);
2020
var filterRegistration = GlobalSettings.RegisterFilter(filter);
2121

@@ -55,7 +55,7 @@ public void CorrectlyEncodesAndDecodesInput()
5555
const string decodedInput = "This is a substitution cipher";
5656
const string encodedInput = "Guvf vf n fhofgvghgvba pvcure";
5757

58-
var attributes = new List<FilterAttributeEntry> { new FilterAttributeEntry("filter=rot13") };
58+
var attributes = new List<FilterAttributeEntry> { new FilterAttributeEntry("rot13") };
5959
var filter = new SubstitutionCipherFilter("cipher-filter", attributes);
6060
var filterRegistration = GlobalSettings.RegisterFilter(filter);
6161

@@ -94,9 +94,9 @@ public void CorrectlyEncodesAndDecodesInput()
9494
[InlineData("*.txt", ".txt", 1, 0)]
9595
public void WhenStagedFileDoesNotMatchPathSpecFileIsNotFiltered(string pathSpec, string fileExtension, int cleanCount, int smudgeCount)
9696
{
97-
const string filterName = "filter=rot13";
97+
const string filterName = "rot13";
9898
const string decodedInput = "This is a substitution cipher";
99-
string attributeFileEntry = string.Format("{0} {1}", pathSpec, filterName);
99+
string attributeFileEntry = string.Format("{0} filter={1}", pathSpec, filterName);
100100

101101
var filterForAttributes = new List<FilterAttributeEntry> { new FilterAttributeEntry(filterName) };
102102
var filter = new SubstitutionCipherFilter("cipher-filter", filterForAttributes);
@@ -122,15 +122,13 @@ public void WhenStagedFileDoesNotMatchPathSpecFileIsNotFiltered(string pathSpec,
122122
}
123123

124124
[Theory]
125-
[InlineData("filter=rot13", "*.txt filter=rot13", 1)]
126-
[InlineData("filter=rot13", "*.txt filter=fake", 0)]
127-
[InlineData("filter=rot13", "*.bat filter=rot13", 0)]
128125
[InlineData("rot13", "*.txt filter=rot13", 1)]
129126
[InlineData("rot13", "*.txt filter=fake", 0)]
127+
[InlineData("rot13", "*.bat filter=rot13", 0)]
128+
[InlineData("rot13", "*.txt filter=fake", 0)]
130129
[InlineData("fake", "*.txt filter=fake", 1)]
131-
[InlineData("filter=fake", "*.txt filter=fake", 1)]
132-
[InlineData("filter=fake", "*.bat filter=fake", 0)]
133-
[InlineData("filter=rot13", "*.txt filter=rot13 -crlf", 1)]
130+
[InlineData("fake", "*.bat filter=fake", 0)]
131+
[InlineData("rot13", "*.txt filter=rot13 -crlf", 1)]
134132
public void CleanIsCalledIfAttributeEntryMatches(string filterAttribute, string attributeEntry, int cleanCount)
135133
{
136134
const string decodedInput = "This is a substitution cipher";
@@ -158,12 +156,10 @@ public void CleanIsCalledIfAttributeEntryMatches(string filterAttribute, string
158156
}
159157

160158
[Theory]
161-
[InlineData("filter=rot13", "*.txt filter=rot13", 1)]
162-
[InlineData("filter=rot13", "*.txt filter=fake", 0)]
163-
[InlineData("filter=rot13", "*.bat filter=rot13", 0)]
159+
164160
[InlineData("rot13", "*.txt filter=rot13", 1)]
165161
[InlineData("rot13", "*.txt filter=fake", 0)]
166-
[InlineData("filter=rot13", "*.txt filter=rot13 -crlf", 1)]
162+
[InlineData("rot13", "*.txt filter=rot13 -crlf", 1)]
167163
public void SmudgeIsCalledIfAttributeEntryMatches(string filterAttribute, string attributeEntry, int smudgeCount)
168164
{
169165
const string decodedInput = "This is a substitution cipher";

‎LibGit2Sharp/Core/Ensure.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,33 @@ public static void ArgumentDoesNotContainZeroByte(string argumentValue, string a
8787
"Zero bytes ('\\0') are not allowed. A zero byte has been found at position {0}.", zeroPos), argumentName);
8888
}
8989

90+
/// <summary>
91+
/// Checks an argument to ensure it isn't a IntPtr.Zero (aka null).
92+
/// </summary>
93+
/// <param name="argumentValue">The argument value to check.</param>
94+
/// <param name="argumentName">The name of the argument.</param>
95+
public static void ArgumentNotZeroIntPtr(IntPtr argumentValue, string argumentName)
96+
{
97+
if (argumentValue == IntPtr.Zero)
98+
{
99+
throw new ArgumentNullException(argumentName);
100+
}
101+
}
102+
103+
/// <summary>
104+
/// Checks a pointer argument to ensure it is the expected pointer value.
105+
/// </summary>
106+
/// <param name="argumentValue">The argument value to check.</param>
107+
/// <param name="expectedValue">The expected value.</param>
108+
/// <param name="argumentName">The name of the argument.</param>
109+
public static void ArhumentIsExpectedIntPtr(IntPtr argumentValue, IntPtr expectedValue, string argumentName)
110+
{
111+
if (argumentValue != expectedValue)
112+
{
113+
throw new ArgumentException("Unexpected IntPtr value", argumentName);
114+
}
115+
}
116+
90117
private static readonly Dictionary<GitErrorCode, Func<string, GitErrorCode, GitErrorCategory, LibGit2SharpException>>
91118
GitErrorsToLibGit2SharpExceptions =
92119
new Dictionary<GitErrorCode, Func<string, GitErrorCode, GitErrorCategory, LibGit2SharpException>>

‎LibGit2Sharp/Core/GitWriteStream.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ internal class GitWriteStream
1515
[MarshalAs(UnmanagedType.FunctionPtr)]
1616
public free_fn free;
1717

18-
public delegate int write_fn(IntPtr stream, IntPtr buffer, uint len);
18+
public delegate int write_fn(IntPtr stream, IntPtr buffer, UIntPtr len);
1919
public delegate int close_fn(IntPtr stream);
2020
public delegate void free_fn(IntPtr stream);
2121
}

‎LibGit2Sharp/Core/NativeMethods.cs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,12 +191,6 @@ internal static extern int git_branch_remote_name(
191191
RepositorySafeHandle repo,
192192
[MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictUtf8Marshaler))] string canonical_branch_name);
193193

194-
[DllImport(libgit2)]
195-
internal static extern int git_buf_grow(IntPtr buffer, UIntPtr targetSize);
196-
197-
[DllImport(libgit2)]
198-
internal static extern int git_buf_put(IntPtr buffer, IntPtr data, UIntPtr len);
199-
200194
[DllImport(libgit2)]
201195
internal static extern int git_remote_rename(
202196
ref GitStrArray problems,

‎LibGit2Sharp/Core/Proxy.cs

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -241,33 +241,6 @@ public static string git_branch_upstream_name(RepositorySafeHandle handle, strin
241241

242242
#region git_buf_
243243

244-
public static void git_buf_grow(IntPtr gitBufPointer, ulong target_size)
245-
{
246-
using (ThreadAffinity())
247-
{
248-
var res = NativeMethods.git_buf_grow(gitBufPointer, (UIntPtr)target_size);
249-
Ensure.ZeroResult(res);
250-
}
251-
}
252-
253-
public static void git_buf_put(IntPtr gitBufPointer, byte[] data, int offset, int count)
254-
{
255-
using (ThreadAffinity())
256-
{
257-
unsafe
258-
{
259-
int res;
260-
261-
fixed (byte* ptr = data)
262-
{
263-
res = NativeMethods.git_buf_put(gitBufPointer, (IntPtr)ptr, (UIntPtr)count);
264-
}
265-
266-
Ensure.ZeroResult(res);
267-
}
268-
}
269-
}
270-
271244
public static void git_buf_free(GitBuf buf)
272245
{
273246
NativeMethods.git_buf_free(buf);
@@ -881,10 +854,7 @@ public static FilterMode git_filter_source_mode(IntPtr filterSource)
881854

882855
public static void git_filter_free(IntPtr gitFilter)
883856
{
884-
using (ThreadAffinity())
885-
{
886-
NativeMethods.git_filter_free(gitFilter);
887-
}
857+
NativeMethods.git_filter_free(gitFilter);
888858
}
889859

890860
#endregion

‎LibGit2Sharp/Filter.cs

Lines changed: 86 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ protected Filter(string name, IEnumerable<FilterAttributeEntry> attributes)
4141
{
4242
attributes = EncodingMarshaler.FromManaged(Encoding.UTF8, attributesAsString),
4343
init = InitializeCallback,
44-
stream = StreamCallback,
44+
stream = StreamCreateCallback,
4545
};
4646
}
4747

@@ -199,129 +199,140 @@ int InitializeCallback(IntPtr filterPointer)
199199
return Initialize();
200200
}
201201

202-
unsafe int StreamCallback(out IntPtr git_writestream_out, GitFilter self, IntPtr payload, IntPtr filterSourcePtr, IntPtr git_writestream_next)
202+
int StreamCreateCallback(out IntPtr git_writestream_out, GitFilter self, IntPtr payload, IntPtr filterSourcePtr, IntPtr git_writestream_next)
203203
{
204-
if (filterSourcePtr == IntPtr.Zero)
204+
int result = 0;
205+
206+
try
205207
{
206-
throw new ArgumentNullException("filterSourcePtr");
208+
Ensure.ArgumentNotZeroIntPtr(filterSourcePtr, "filterSourcePtr");
209+
Ensure.ArgumentNotZeroIntPtr(git_writestream_next, "git_writestream_next");
210+
211+
thisStream = new GitWriteStream();
212+
thisStream.close = StreamCloseCallback;
213+
thisStream.write = StreamWriteCallback;
214+
thisStream.free = StreamFreeCallback;
215+
thisPtr = Marshal.AllocHGlobal(Marshal.SizeOf(thisStream));
216+
Marshal.StructureToPtr(thisStream, thisPtr, false);
217+
nextPtr = git_writestream_next;
218+
nextStream = new GitWriteStream();
219+
Marshal.PtrToStructure(nextPtr, nextStream);
220+
filterSource = FilterSource.FromNativePtr(filterSourcePtr);
207221
}
208-
if (git_writestream_next == IntPtr.Zero)
222+
catch (Exception exception)
209223
{
210-
throw new ArgumentNullException("git_writestream_next");
211-
}
224+
// unexpected failures means memory clean up required
225+
if (thisPtr != IntPtr.Zero)
226+
{
227+
Marshal.FreeHGlobal(thisPtr);
228+
thisPtr = IntPtr.Zero;
229+
}
212230

213-
thisStream = new GitWriteStream();
214-
thisStream.close = StreamCloseCallback;
215-
thisStream.write = StreamWriteCallback;
216-
thisStream.free = StreamFreeCallback;
217-
thisPtr = Marshal.AllocHGlobal(Marshal.SizeOf(thisStream));
218-
Marshal.StructureToPtr(thisStream, thisPtr, false);
219-
nextPtr = git_writestream_next;
220-
nextStream = new GitWriteStream();
221-
Marshal.PtrToStructure(nextPtr, nextStream);
222-
filterSource = FilterSource.FromNativePtr(filterSourcePtr);
231+
Proxy.giterr_set_str(GitErrorCategory.Filter, exception.Message);
232+
result = (int)GitErrorCode.Error;
233+
}
223234

224235
git_writestream_out = thisPtr;
225236

226-
return 0;
237+
return result;
227238
}
228239

229-
unsafe int StreamCloseCallback(IntPtr stream)
240+
int StreamCloseCallback(IntPtr stream)
230241
{
231242
int result = 0;
232243

233-
if (stream == IntPtr.Zero)
234-
{
235-
throw new ArgumentNullException("stream");
236-
}
237-
if (stream != thisPtr)
244+
try
238245
{
239-
throw new ArgumentException("Unexpected stream pointer", "stream");
240-
}
246+
Ensure.ArgumentNotZeroIntPtr(stream, "stream");
247+
Ensure.ArhumentIsExpectedIntPtr(stream, thisPtr, "stream");
241248

242-
using (MemoryStream output = new MemoryStream())
249+
result = nextStream.close(nextPtr);
250+
}
251+
catch (Exception exception)
243252
{
244-
Ensure.ZeroResult(result = this.Complete(filterSource.Path, filterSource.Root, output));
245-
result = WriteToNextFilter(output);
253+
Proxy.giterr_set_str(GitErrorCategory.Filter, exception.Message);
254+
result = (int)GitErrorCode.Error;
246255
}
247256

248-
return nextStream.close(nextPtr);
257+
return result;
249258
}
250259

251-
unsafe void StreamFreeCallback(IntPtr stream)
260+
void StreamFreeCallback(IntPtr stream)
252261
{
253-
if (stream == IntPtr.Zero)
254-
throw new ArgumentNullException("stream");
255-
if (stream != thisPtr)
256-
throw new ArgumentException("unexpected stream ptr");
262+
try
263+
{
264+
Ensure.ArgumentNotZeroIntPtr(stream, "stream");
265+
Ensure.ArhumentIsExpectedIntPtr(stream, thisPtr, "stream");
257266

258-
Marshal.FreeHGlobal(thisPtr);
267+
Marshal.FreeHGlobal(thisPtr);
268+
}
269+
catch { }
259270
}
260271

261-
unsafe int StreamWriteCallback(IntPtr stream, IntPtr buffer, uint len)
272+
unsafe int StreamWriteCallback(IntPtr stream, IntPtr buffer, UIntPtr len)
262273
{
263274
int result = 0;
264275

265-
if (stream == IntPtr.Zero)
266-
{
267-
throw new ArgumentNullException("stream");
268-
}
269-
if (buffer == IntPtr.Zero)
270-
{
271-
throw new ArgumentNullException("buffer");
272-
}
273-
if (stream != thisPtr)
276+
try
274277
{
275-
throw new ArgumentException("Unexpected GitWriteStream", "stream");
276-
}
278+
Ensure.ArgumentNotZeroIntPtr(stream, "stream");
279+
Ensure.ArgumentNotZeroIntPtr(buffer, "buffer");
280+
Ensure.ArhumentIsExpectedIntPtr(stream, thisPtr, "stream");
277281

278-
using (UnmanagedMemoryStream input = new UnmanagedMemoryStream((byte*)buffer.ToPointer(), len))
279-
using (MemoryStream output = new MemoryStream())
280-
{
281-
switch (filterSource.SourceMode)
282+
using (UnmanagedMemoryStream input = new UnmanagedMemoryStream((byte*)buffer.ToPointer(), (long)len))
283+
using (MemoryStream output = new MemoryStream())
282284
{
283-
case FilterMode.Clean:
284-
result = Clean(filterSource.Path, filterSource.Root, input, output);
285-
break;
286-
case FilterMode.Smudge:
287-
result = Smudge(filterSource.Path, filterSource.Root, input, output);
288-
break;
289-
default:
290-
Proxy.giterr_set_str(GitErrorCategory.Filter, "Unexpected filter mode.");
291-
return (int)GitErrorCode.Ambiguous;
292-
}
293285

294-
if (result == (int)GitErrorCode.PassThrough)
295-
{
296-
input.CopyTo(output);
297-
}
298-
else if (result < 0)
299-
{
300-
return result;
301-
}
286+
switch (filterSource.SourceMode)
287+
{
288+
case FilterMode.Clean:
289+
result = Clean(filterSource.Path, filterSource.Root, input, output);
290+
break;
291+
case FilterMode.Smudge:
292+
result = Smudge(filterSource.Path, filterSource.Root, input, output);
293+
break;
294+
default:
295+
Proxy.giterr_set_str(GitErrorCategory.Filter, "Unexpected filter mode.");
296+
return (int)GitErrorCode.Ambiguous;
297+
}
298+
299+
if (result == (int)GitErrorCode.PassThrough)
300+
{
301+
input.CopyTo(output);
302+
}
303+
else if (result < 0)
304+
{
305+
return result;
306+
}
302307

303-
result = WriteToNextFilter(output);
308+
output.Seek(0, SeekOrigin.Begin);
309+
result = WriteToNextFilter(output);
310+
}
311+
}
312+
catch (Exception exception)
313+
{
314+
Proxy.giterr_set_str(GitErrorCategory.Filter, exception.Message);
315+
result = (int)GitErrorCode.Error;
304316
}
305317

306318
return result;
307319
}
308320

309321
private unsafe int WriteToNextFilter(MemoryStream output)
310322
{
311-
const int BufferSize = 64 * 1024; // 64K is optimal buffer size per https://technet.microsoft.com/en-us/library/cc938632.aspx
323+
// 64K is optimal buffer size per https://technet.microsoft.com/en-us/library/cc938632.aspx
324+
const int BufferSize = 64 * 1024;
312325

313326
int result = 0;
314327
byte[] bytes = new byte[BufferSize];
315328
IntPtr bytesPtr = Marshal.AllocHGlobal(BufferSize);
316329
try
317330
{
318-
output.Seek(0, SeekOrigin.Begin);
319-
320331
int read = 0;
321332
while ((read = output.Read(bytes, 0, bytes.Length)) > 0)
322333
{
323334
Marshal.Copy(bytes, 0, bytesPtr, read);
324-
if ((result = nextStream.write(nextPtr, bytesPtr, (uint)read)) < 0)
335+
if ((result = nextStream.write(nextPtr, bytesPtr, (UIntPtr)read)) < 0)
325336
{
326337
Proxy.giterr_set_str(GitErrorCategory.Filter, "Filter write to next stream failed");
327338
break;

0 commit comments

Comments
 (0)