Merge pull request robbiehanson#205 from talko/availspacebugs

dvor · dvor · commit d50544e78ee5 · 2014-02-10T20:45:53.000+02:00
Failing unit test and fix for buffer overrun access violation in GCDAsyncSocketPreBuffer.
diff --git a/GCD/GCDAsyncSocket.m b/GCD/GCDAsyncSocket.m
@@ -385,7 +385,7 @@ - (void)dealloc
 
 - (void)ensureCapacityForWrite:(size_t)numBytes
 {
-	size_t availableSpace = preBufferSize - (writePointer - readPointer);
+	size_t availableSpace = [self availableSpace];
 	
 	if (numBytes > availableSpace)
 	{
@@ -418,7 +418,7 @@ - (uint8_t *)readBuffer
 - (void)getReadBuffer:(uint8_t **)bufferPtr availableBytes:(size_t *)availableBytesPtr
 {
 	if (bufferPtr) *bufferPtr = readPointer;
-	if (availableBytesPtr) *availableBytesPtr = writePointer - readPointer;
+	if (availableBytesPtr) *availableBytesPtr = [self availableBytes];
 }
 
 - (void)didRead:(size_t)bytesRead
@@ -435,7 +435,7 @@ - (void)didRead:(size_t)bytesRead
 
 - (size_t)availableSpace
 {
-	return preBufferSize - (writePointer - readPointer);
+	return preBufferSize - (writePointer - preBuffer);
 }
 
 - (uint8_t *)writeBuffer
@@ -446,7 +446,7 @@ - (uint8_t *)writeBuffer
 - (void)getWriteBuffer:(uint8_t **)bufferPtr availableSpace:(size_t *)availableSpacePtr
 {
 	if (bufferPtr) *bufferPtr = writePointer;
-	if (availableSpacePtr) *availableSpacePtr = preBufferSize - (writePointer - readPointer);
+	if (availableSpacePtr) *availableSpacePtr = [self availableSpace];
 }
 
 - (void)didWrite:(size_t)bytesWritten
diff --git a/GCD/Xcode/TestPreBuffer/Shared/TestPreBuffer.m b/GCD/Xcode/TestPreBuffer/Shared/TestPreBuffer.m
@@ -59,7 +59,7 @@ + (void)start
 	// Run unit tests
 	
 	[self test_preBuffer];
-	
+
 	// Setup benchmarks.
 	// 
 	// We're going to test a common pattern within GCDAsyncSocket, which is:
@@ -89,7 +89,7 @@ + (void)test_preBuffer
 	// Test capacity, and initial size methods
 	
 	size_t capacity = [preBuffer availableSpace];
-	
+
 	NSAssert([preBuffer availableSpace] >= 1024, @"1A");
 	NSAssert([preBuffer availableBytes] == 0, @"1B");
 	
@@ -101,7 +101,7 @@ + (void)test_preBuffer
 	writePointer1 = [preBuffer writeBuffer];
 	[preBuffer didWrite:512];
 	writePointer2 = [preBuffer writeBuffer];
-	
+
 	NSAssert(writePointer2 - writePointer1 == 512, @"2A");
 	NSAssert([preBuffer availableBytes] == 512, @"2B");
 	
@@ -113,15 +113,19 @@ + (void)test_preBuffer
 	readPointer1 = [preBuffer readBuffer];
 	[preBuffer didRead:256];
 	readPointer2 = [preBuffer readBuffer];
-	
+
 	NSAssert(readPointer2 - readPointer1 == 256, @"3A");
 	NSAssert([preBuffer availableBytes] == 256, @"3B");
 	
 	[preBuffer didRead:256];
 	
 	NSAssert([preBuffer availableBytes] == 0, @"4A");
 	NSAssert([preBuffer availableSpace] == capacity, @"4B");
-	
+
+    // At this point, the buffer should have reset
+    NSAssert([preBuffer readBuffer] == [preBuffer writeBuffer], @"4C");
+    NSAssert([preBuffer availableSpace] == 1024, @"4D");
+
 	// Test write and read
 	
 	char *str = "test";
@@ -140,7 +144,20 @@ + (void)test_preBuffer
 	NSAssert([preBuffer availableSpace] >= (capacity * 2), @"6A");
 	NSAssert([preBuffer availableBytes] == strLen, @"6B");
 	NSAssert(memcmp([preBuffer readBuffer], str, strLen) == 0, @"6C");
-	
+
+    // Test available space
+    [preBuffer reset];
+    size_t availableSpace = [preBuffer availableSpace];
+
+    // Make sure the available space is correct if we write all but 1 byte of our available space
+    size_t writeCount = availableSpace - 1;
+    [preBuffer didWrite:writeCount];
+    NSAssert([preBuffer availableSpace] == 1, @"7A");
+
+    // Make sure it doesn't change if we read some, but not all, of the data
+    [preBuffer didRead:writeCount - 1];
+    NSAssert([preBuffer availableSpace] == 1, @"7B");
+
 	NSLog(@"%@: passed", NSStringFromSelector(_cmd));
 }
 

Original file line number	Diff line number	Diff line change
`@@ -385,7 +385,7 @@ - (void)dealloc`
`385`	`385`
`386`	`386`	`- (void)ensureCapacityForWrite:(size_t)numBytes`
`387`	`387`	`{`
`388`		`- size_t availableSpace = preBufferSize - (writePointer - readPointer);`
	`388`	`+ size_t availableSpace = [self availableSpace];`
`389`	`389`
`390`	`390`	`if (numBytes > availableSpace)`
`391`	`391`	`{`
`@@ -418,7 +418,7 @@ - (uint8_t *)readBuffer`
`418`	`418`	`- (void)getReadBuffer:(uint8_t *)bufferPtr availableBytes:(size_t )availableBytesPtr`
`419`	`419`	`{`
`420`	`420`	`if (bufferPtr) *bufferPtr = readPointer;`
`421`		`- if (availableBytesPtr) *availableBytesPtr = writePointer - readPointer;`
	`421`	`+ if (availableBytesPtr) *availableBytesPtr = [self availableBytes];`
`422`	`422`	`}`
`423`	`423`
`424`	`424`	`- (void)didRead:(size_t)bytesRead`
`@@ -435,7 +435,7 @@ - (void)didRead:(size_t)bytesRead`
`435`	`435`
`436`	`436`	`- (size_t)availableSpace`
`437`	`437`	`{`
`438`		`- return preBufferSize - (writePointer - readPointer);`
	`438`	`+ return preBufferSize - (writePointer - preBuffer);`
`439`	`439`	`}`
`440`	`440`
`441`	`441`	`- (uint8_t *)writeBuffer`
`@@ -446,7 +446,7 @@ - (uint8_t *)writeBuffer`
`446`	`446`	`- (void)getWriteBuffer:(uint8_t *)bufferPtr availableSpace:(size_t )availableSpacePtr`
`447`	`447`	`{`
`448`	`448`	`if (bufferPtr) *bufferPtr = writePointer;`
`449`		`- if (availableSpacePtr) *availableSpacePtr = preBufferSize - (writePointer - readPointer);`
	`449`	`+ if (availableSpacePtr) *availableSpacePtr = [self availableSpace];`
`450`	`450`	`}`
`451`	`451`
`452`	`452`	`- (void)didWrite:(size_t)bytesWritten`